[Mesa-dev] [PATCH 2/2] glsl: allow linking ES shaders with desktop shaders in desktop GL

[Timothy Arceri]

On 17/5/18 10:55 am, Ilia Mirkin wrote:
> ES has all kinds of additional linking rules. I suspect they'll break
> down if some of the shaders are ES and some are not-ES.

Hi Ilia,

Can you think of an example of this. I've attempted to create a CTS test 
with the following justification:


Add linking test for mixing of GLSL and GLSL ES shaders

The ARB_ES2_compatibility spec doesn't explicitly state the mixing
of these shaders is not allowed. However the strict rules around
GLSL ES shader versions when linking implies this is not allowed.

 From the ARB_ES2_compatibility spec:

    "(6) What should we do about conflicting #version values?

     RESOLVED: The original GLSL 1.00 spec did not include #version,
     it was added in GLSL 1.10 and the first accepted value was
     "110". GLSL ES has only one version and it is "100", so there
     is currently no overlap and ambiguity. So GLSL can be extended
     to accept "100" and always interpret it to mean "GLSL ES 1.00
     functionality"."

 From the OpenGL ES 3.2 spec:

    "Linking can fail for a variety of reasons as specified in the
     OpenGL ES Shading Language Specification, as well as any of
     the following reasons:

           ...

        - The shaders do not use the same shader language version."

However the Nvidia guys are not buying it.

"I don't think this is a valid "OpenGL" conformance test since the 
OpenGL spec doesn't forbid mixing shader versions, including ESSL 
shaders. This would be a valid "OpenGL ES" conformance test.

If you think the OpenGL spec is wrong, please file an issue at 
https://gitlab.khronos.org/opengl/API."

If we can come up with a good example of why this should not be allowed 
I'll file a spec bug. Otherwise maybe this patch is valid after all.

> 
> This all does sound very odd -- what did you see in practice?
> 
> On Wed, May 16, 2018 at 3:04 AM, Timothy Arceri <tarceri at itsqueeze.com> wrote:
>> In GLES shader versions must match but there is nothing
>> in the ARB_ES*_compatibility specs that say they must match.
>>
>> This fixes some compilation errors in Google Earth VR.
>> ---
>>   src/compiler/glsl/linker.cpp | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/compiler/glsl/linker.cpp b/src/compiler/glsl/linker.cpp
>> index f060c5316fa..2b7ee0ad5a6 100644
>> --- a/src/compiler/glsl/linker.cpp
>> +++ b/src/compiler/glsl/linker.cpp
>> @@ -83,6 +83,7 @@
>>   #include "builtin_functions.h"
>>   #include "shader_cache.h"
>>
>> +#include "main/context.h"
>>   #include "main/imports.h"
>>   #include "main/shaderobj.h"
>>   #include "main/enums.h"
>> @@ -4799,7 +4800,8 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>>         min_version = MIN2(min_version, prog->Shaders[i]->Version);
>>         max_version = MAX2(max_version, prog->Shaders[i]->Version);
>>
>> -      if (prog->Shaders[i]->IsES != prog->Shaders[0]->IsES) {
>> +      if (!_mesa_is_desktop_gl(ctx) &&
>> +          prog->Shaders[i]->IsES != prog->Shaders[0]->IsES) {
>>            linker_error(prog, "all shaders must use same shading "
>>                         "language version\n");
>>            goto done;
>> @@ -4817,7 +4819,7 @@ link_shaders(struct gl_context *ctx, struct gl_shader_program *prog)
>>      /* In desktop GLSL, different shader versions may be linked together.  In
>>       * GLSL ES, all shader versions must be the same.
>>       */
>> -   if (prog->Shaders[0]->IsES && min_version != max_version) {
>> +   if (!_mesa_is_desktop_gl(ctx) && min_version != max_version) {
>>         linker_error(prog, "all shaders must use same shading "
>>                      "language version\n");
>>         goto done;
>> --
>> 2.17.0
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>