PDA

View Full Version : getAttribLocation fix suggestion



elanthis
11-14-2011, 02:37 PM
Having just helped yet another round of upcoming graphics devs work around the broken behavior of getAttribLocation (and it's uniform cousin), I have one simple but critical suggestion for the OpenGL API:

Don't return an error code when requesting the location of an unused attribute or uniform.

Seriously, it's a huge pain in the ass. It confuses almost every OpenGL newcomer I've ever met. It makes writing debugging layers impossible, too, as the C API has no way to inform the user if an attribute name was misspelled or if some shader code is just commented or stubbed out.

I realize that allocating input assembly slots may not be ideal for shaders that legitimately have a ton of unused attributes based on compile-time conditions. I'd suggest a new return value for the get*Location calls to represent "exists but no valid input location.". Bonus points for making that value be a legal input to the AttribArray functions that results in a no-op.

Alfonse Reinheart
11-14-2011, 02:59 PM
Don't return an error code when requesting the location of an unused attribute or uniform.

... OK, so what should it return? A valid location that refers to nothing? How would you tell the difference?

And what do you do with the mountains of code that already exists?

It's interesting how you add "uniform" here, and yet the entire rest of your text is squarely focused on attributes. Maybe that's because all of the `glUniform` functions accept -1 and just drop the value. If so, then why request it for uniforms at all?


Seriously, it's a huge pain in the ass. It confuses almost every OpenGL newcomer I've ever met. It makes writing debugging layers impossible, too, as the C API has no way to inform the user if an attribute name was misspelled or if some shader code is just commented or stubbed out.

People still use glGetAttribLocation? I thought that went the way of the Dodo the very day explicit_attrib_location (http://www.opengl.org/registry/specs/ARB/explicit_attrib_location.txt) came out.

Indeed, I never used it. Before explicit_attrib_location, I just used glBindAttribLocation to set it manually. I even had a generic setup function that would take a program and bind all of the commonly used names to it. You have everything to gain by establishing an attribute index convention, and nothing to lose by it.

In any case, the ARB is not going to radically alter the functioning of already existing applications, especially since that would break those aforementioned existing applications.

elanthis
11-15-2011, 02:32 AM
... OK, so what should it return? A valid location that refers to nothing? How would you tell the difference?

Magic values are not unheard of, especially in crusfty C APIs using integers for handles. :) You're most likely right that no value could be chosen that wouldn't break some app in some case out there, though.


It's interesting how you add "uniform" here, and yet the entire rest of your text is squarely focused on attributes.

I was typing on an iPhone, wanted to avoid repeating a bunch of "and uniforms" all over the place. Nothing more spectacular than that, but thanks for making it look like some kind of conspiracy. :)


Maybe that's because all of the `glUniform` functions accept -1 and just drop the value. If so, then why request it for uniforms at all?

Same problems: debugging. Ignoring a -1 result in the attribute binding functions could be trivial (in fact that's all that most of the wrappers I see students write do).

Telling the developer _why_ he got that is not so trivial; in fact, impossible with the current GL API. In usual OpenGL fashion, there are multiple possible causes of failure, all of which return GL_INVALID_OPERATION as the error code. I'm actually not sure if there even are any other GL error codes, honestly. $deity knows I've never seen one.

During development and debugging -- especially for newcomers to GL, and not people who've been around for 200 grumpy years like yourself -- being able to easily tell the difference between "I made a typo in my shader," "I made a typo in my application code," and "GL is just screwing with me and silently throwing away things I declared in my shader" is pretty handy.

... I suppose now that I think on it, just adding a new GL error code would be just as good as adding a new magic return value, would be easier, and is far far less likely to break any existing code. I amend my suggestion to just doing that instead. GL_UNKNOWN_ATTRIBUTE if the name is not found, and GL_IGNORED_ATTRIBUTE or something similar if a shader declared it but the linker removed it. (Teeny little bit of extra metadata on the program object goes a long way here.)


People still use glGetAttribLocation? I thought that went the way of the Dodo the very day explicit_attrib_location came out.

In "real world" apps, sure. Most of the teaching materials I see are lucky if they're not still using immediate mode rendering. Sigh. (And yes, immediate mode rendering _and_ shaders. Academia is able to keep up with basic advances in hardware; but only barely, it seems.) Admittedly, the GL documentation scene is pretty sorry: there's (1) horrendously outdated crap, (2) horrendously incorrect or poorly written crap, and (3) texts assuming a mastery of calculus, linear algebra, discrete math, Z80 assembly, higher-order geometrical functions, catalytic converters, and some wildly inappropriate language like Scheme or Prolog or Forth or Java that nobody aside from people writing research papers would even think of using for modern graphics applications. So yeah, you see an awful lot of glGetAttribLocation when you're new to GL. (Also, GLES doesn't have explicit attribute location support at all, does it? That's become more relevant, even to students, thanks to things like WebGL and the popularity of mobile game development.)

I'd argue that even the bind-by-location API could use some extra features to make debugging easier. glProgramHasAttribLocation() maybe ? glShaderGetAttribType() ? glDidYouOptimizeOutMyCodeBecauseICommentedOutSomeO therLinesOfCodeWhileDebugging() ? glTranslateInvalidOperationToAMeaningfulErrorCode( ) ? (okay, the first two at least were real suggestions.)

Really though, a little bit of thought into little niceties that helps the API user out during the more frustrating parts of development makes the difference between nice APIs and GL.

Pleeeeease?

Alfonse Reinheart
11-15-2011, 12:05 PM
In usual OpenGL fashion, there are multiple possible causes of failure, all of which return GL_INVALID_OPERATION as the error code. I'm actually not sure if there even are any other GL error codes, honestly. $deity knows I've never seen one.

... what? First, if you don't actually know OpenGL (http://www.opengl.org/wiki/GL_Error_Codes), then making suggestions for it is probably not an appropriate thing for you to do.

Second, you cannot possibly have ever run into the problem you suggest, because passing -1 to glVertexAttribPointer results in GL_INVALID_VALUE, not OPERATION. So you cannot both have never seen any other error and have run into the problem you're complaining about.

And if this is just ridiculous hyperbole, stop using hyperbole when making a serious suggestion. It makes your "suggestion" sound less like a suggestion and more like grousing.


So yeah, you see an awful lot of glGetAttribLocation when you're new to GL.

Then fix that. Users should not be taught to use `glGetAttribLocation`. They should be taught to tell OpenGL what attribute locations they want and thereby establish an attribute convention.

For example, my tutorials (linked in my sig) are for new users, but they never once call `glGetAttribLocation`. And even before explicit_attrib_location, I just used `glBindAttribLocation` before linking in those tutorials. That is how it should be done, that is how people actually do it, and therefore, that is how people should be taught to do it.

Tutorials and learning materials that suggest using `glGetAttribLocation` are wrong and should be corrected.


Also, GLES doesn't have explicit attribute location support at all, does it?

No, but it does have glBindAttribLocation. Why are you getting something that you've already told the program to set up? And if you didn't set it up, then that is what you should be fixing.


I suppose now that I think on it, just adding a new GL error code would be just as good as adding a new magic return value, would be easier, and is far far less likely to break any existing code. I amend my suggestion to just doing that instead.

I want to make sure I understand your suggestion. You're saying that if you call `glGetAttribLocation` for an attribute that is either inactive or doesn't exist, this should throw a GL Error.

Even though this is not an error. Even though errors are used to indicate erroneous conditions or uses of a function, yet it is perfectly valid to ask for attribute locations that were optimized out. And even those all of those tutorials and whatnot that you cite will not be affected by this in the slightest, because virtually all of them ignore error checking entirely.

So who exactly is this feature for, anyway?


glProgramHasAttribLocation() maybe ?

Inactive attributes don't have a location. What you probably intended is, "glProgramIsAttribInactive", which would return false for attributes that aren't on the inactive list. You would use it like this:



GLint loc = glGetAttribLocation(...);
if(loc == -1)
{
if(glProgramIsAttribInactive(...))
//Inactive attribute.
else
//Name mismatch.
}



glShaderGetAttribType() ?

... what?

If you are going to ask for something, it would be a good idea to at least make a cursory inspection of the API to make sure that it doesn't already exist. (http://www.opengl.org/sdk/docs/man3/xhtml/glGetActiveAttrib.xml)

And before you ask, there's no reason to ask for information for an inactive attribute. So it's find that it only works for active ones.