Bug 35425

The PipeCap for instanced drawing is available, the drivers implement them, TGSI has the construct TGSI_SEMANTIC_INSTANCEID.

The only component that does not support instanced drawing is the GLSL compiler.

So can please any of the GLSL guys implement the gl_InstanceID uniform?

Created attachment 45367 [details] [review]
Probable patch

This patch works for me, but i dont know if it fits the coding style, so take the best bytes of it to fix the problem

The application is *BROKEN*.  gl_InstanceID is *NOT* part of GL_ARB_draw_instanced.  The correct name is gl_InstanceIDARB.  Look at the spec (http://www.opengl.org/registry/specs/ARB/draw_instanced.txt).  I'm not going to deviate from the spec to work around broken apps.

The name gl_InstanceID comes from either GLSL 1.40 or GL_EXT_gpu_shader4.  We don't support either of those.  The GL_EXT_draw_instanced spec even mentions this:

  (1) Should instanceID be provided by this extension, or should it be
      provided by EXT_gpu_shader4, thus creating a dependence on that
      spec?

        Resolved: While this extension could stand alone, its utility
        would be limited without the additional functionality provided
        by EXT_gpu_shader4; also, the spec language is cleaner if
        EXT_gpu_shader4 assumes instanceID is always available, even
        if its value is always zero without this extension.

I already NAKed this patch when it was posted to the mailing list.  See http://marc.info/?l=mesa3d-dev&m=130014281021091&w=2.  Here's what I said there:

NAK for a couple reasons.

This extension depends on EXT_gpu_shader4, which we don't support.
There's no #extension bit required for this extension because the GLSL
changes are implemented by EXT_gpu_shader4.  Without EXT_draw_instanced,
gl_InstanceID always reads 0.

Note that EXT_gpu_shader4 says that a #extension line is required to use
the features of the extension, including gl_InstanceID.  If their driver
allows gl_InstanceID without a #extension line, it is a bug in their driver.

If OilRush is trying to use EXT_gpu_shader4 features without enabling
EXT_gpu_shader4 (or at least checking for the extension!) or using GLSL
1.30, it's a bug in OilRush.

In addition, this patch enables gl_InstanceID even if the driver doesn't
support it.

(In reply to comment #3)
> The application is *BROKEN*.  gl_InstanceID is *NOT* part of
> GL_ARB_draw_instanced.  The correct name is gl_InstanceIDARB.

Due to this, it would be safer not to advertise ARB_draw_instanced without EXT_gpu_shader4. I haven't seen an app that uses gl_InstanceIDARB when it should. All apps based on Unigine Engine check for ARB_draw_instanced and use gl_InstanceID.

It's pointless to follow the GL spec if no app follows it either.

My 2 eurocents.

I also share Marek Olšáks opinion.
The mesa driver is ment to be a 3D driver to play games and use 3D application and not to implement a paper-standard.
Just imagine what would happen if browsers would stop parsing if the document would not follow the strict XHTML 1.0 rules. Would you use such a browser?

Your opinion is noted.

The GL_ARB_draw_instanced spec is really broken.  In various ways, it does not match what any vendor ships.  The ARB is working on fixing the spec to match a subset of reality that is useful to developers.  Once consensus is reached, there will almost certainly be changes required to Mesa.

Here's a table of the known problems:

                                             AMD   Mesa   NVIDIA
-------------------------------------------+-----+------+-------
gl_InstanceIDARB must be available         | yes | yes  |  NO
-------------------------------------------+-----+------+-------
gl_InstanceID must not be available        | NO  | yes  |  NO
-------------------------------------------+-----+------+-------
Neither can be available if OpenGL 3.0 nor | N/A |  NO  |  N/A
GL_EXT_gpu_shader4 are not supported       |     |      |
-------------------------------------------+-----+------+-------

I'll reopen this bug once the spec issues are resolved.  My guess is that gl_InstanceID will be added and the GL 3.0 / GL_EXT_gpu_shader4 dependency will be dropped.  It is also possible that, alas, gl_InstanceIDARB will be removed too.  We'll see what happens.

You could apply the patch with the commit text "started implementing EXT_gpu_shader4 in GLSL: gl_InstanceID" (with some compatibility checks of course)