Skip to content
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

Ill-specified interaction of display lists and primitive restart #74

Open
ianromanick opened this issue Oct 26, 2020 · 5 comments
Open

Comments

@ianromanick
Copy link

A few bits of the OpenGL 3.1+ specification combine to generate an interaction that may be impossible to satisfy.

  • Starting in OpenGL 3.1, primitive restart enable became server state.
  • glPrimitiveRestartIndex cannot be compiled into a display list.
  • From section 21.4 (Display Lists) of the OpenGL 4.6 Compatibility Profile spec:

Vertex array pointers are dereferenced when the commands ArrayElement, DrawArrays, DrawElements, or DrawRangeElementsare accumulated into a display list.

Based on that, what should the following code do?

    glDisable(GL_PRIMITIVE_RESTART);
    glPrimitiveRestartIndex(0x7fffffff);

    glNewList(1, GL_COMPILE);
    glBegin(GL_TRIANGLES);
    glArrayIndex(0); glArrayIndex(1); glArrayIndex(2);
    glArrayIndex(0x7fffffff);
    glArrayIndex(3); glArrayIndex(4); glArrayIndex(5);
    glEnd();
    glEndList();

    glEnable(GL_PRIMITIVE_RESTART);
    glCallList(1);

    glPrimitiveRestartIndex(0x12345678);
    glCallList(1);

The difficulty is that glArrayIndex(0x7fffffff) is supposed to dereference the vertex array at the time it is called. However, the state of GL_PRIMITIVE_RESTART and the primitive restart index aren't known until glCallList. It is impossible for the implementation to know if 0x7fffffff should start a new primitive or dereference a very large memory address.

On at least one implementation, it crashes inside glArrayIndex(0x7fffffff).

@ianromanick
Copy link
Author

This would also affect GLX implementations of OpenGL 3.1+ compatibility profile due to the need for the client to dereference client-side arrays to send to the server.

@NicolBolas
Copy link

The interaction is not ill-specified. Since the dereferencing happened at the time of compilation, and at the time of the specification, primitive restarting was not set, the behavior is obvious: it accesses the index exactly as you specified it during compilation.

Also, the function is glArrayElement.

@ianromanick
Copy link
Author

Yes, glArrayIndex should have been glArrayElement. You got me on that one.

You have chosen to interpret the spec one way. That interpretation is likely similar to the interpretation made by the vendor that crashes on array index 0x7fffffff. However, at least one other vendor does not interpret the spec that way, and that driver has very different behavior. At the point where two major vendors of desktop OpenGL drivers make dramatically different interpretations of the spec, the spec is, by definition, ill specified.

The problem is that before we removed all of the deprecated functionality from OpenGL 3.1, the primitive restart index and primitive restart control was client state. Since client state was no longer a thing in OpenGL 3.1, the restart index and the restart enable had to change.

In OpenGL 3.0 with GL_NV_primitive_restart, the interactions were all clear, and things just worked. Section 21.4 of the OpenGL 4.6 spec says (earlier versions of the spec say basically the same thing):

Doing so causes the commands within the list to be executed just as if they were given normally. The only exception pertains to commands that rely upon client state. When such a command is accumulated into the display list (that is, when issued, not when executed), the client state in effect at that time applies to the command.

It also says:

Vertex array pointers are dereferenced when the commands ArrayElement, DrawArrays, DrawElements, or DrawRangeElements are accumulated into a display list.

Calls to glClientEnable(GL_PRIMITIVE_RESTART_NV) and glPrimitiveRestartIndexNV modified client state. Those commands cannot be compiled into a display list (more on this below). As client state, this data can affect glDrawElements and friends when they are compiled into the display list. GL_NV_primitive_restart provides glPrimitiveRestartNV for this purpose. When the restart index is encountered either in a display list or when GLX protocol is used, the command glPrimitiveRestartNV is generated instead. Applying the principle of least surprise: the display list usage and the non-display list usage function the same.

Client state affects compilation of display lists, but server state affects execution of display lists. In general, server state is ignored during compilation. The commands are just stored in the list. Section 21.4 calls this out:

If mode is COMPILE, then commands are not executed as they are placed in the display list.

This prevents errors from being generated during compilation. This is why you can make a display list that just contains a bunch of called to glVertex and a trailing call to glEnd. The server state (in this case, whether or not glBegin was called) is ignored. Now GL_PRIMITIVE_RESTART is server state, but it's needed to know how to dereference the data put in the display list. Okay, maybe we could "cheat" and look at the server state while compiling the display list. That only solves part of the problem.

In addition, even in the compatibility profile specs, there is no analog to glPrimitiveRestartNV. Even if the restart index and restart enable issues were clear, the part that would defines how it actually worked in a display list was never incorporated into the core spec. Some commands, such as DrawArraysOneInstance, are defined in the spec just to be used as tools to define other functionality, so it would be perfectly normal to define glPrimitiveRestart in a similar manner.

To make matters worse, glEnable(GL_PRIMITIVE_RESTART) can be compiled into the display list. The behavior of index 0x7fffffff can change from meaning "dereference a huge pointer" to "restart the primitive" in the middle of the display list. Except... that's not possible. The glEnable isn't executed while compiling the display list, but that is when the data is dereferenced. Looking at server state doesn't help here. Somehow we'd have to modify server state too, and that's right out.

As far as I can tell from having done lots and lots of spec archaeology, changing primitive restart enable and primitive restart index to server state in compatibility profile effectively makes it impossible to both follow the letter of the spec and have primitive restart work in display lists. In my opinion, this violates the principle of least surprise. I also don't think it's what anyone intended.

@NicolBolas
Copy link

At the point where two major vendors of desktop OpenGL drivers make dramatically different interpretations of the spec, the spec is, by definition, ill specified.

The existence of driver bugs is not a reasonable definition of "ill specified". "Ill specified" means that the standard does not say what ought to happen. That people might fail to implement it correctly is not de facto evidence of a defect in the standard.

Your point about server state vs. client state is valid, but not the idea that driver bug == ill specified.

@ianromanick
Copy link
Author

I've been actively contributing to OpenGL specs for 20 years. This is the process. Please stop being a troll.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants