[Mesa-dev] [PATCH 4/8] glsl: Add new builtins required by GL_ARB_sample_shading

[Ian Romanick]

On 10/22/2013 07:48 AM, Ian Romanick wrote:
> On 10/16/2013 10:40 AM, Ian Romanick wrote:
>> On 10/15/2013 07:50 PM, Kenneth Graunke wrote:
>>> On 10/15/2013 01:58 PM, Ian Romanick wrote:
>>>> On 10/15/2013 01:50 PM, Anuj Phogat wrote:
>>>>> On Tue, Oct 15, 2013 at 10:02 AM, Ian Romanick <idr at freedesktop.org> wrote:
>>>>>> On 10/14/2013 10:12 AM, Anuj Phogat wrote:
>>>>>>> @@ -789,6 +794,12 @@ builtin_variable_generator::generate_fs_special_vars()
>>>>>>>        if (state->AMD_shader_stencil_export_warn)
>>>>>>>           var->warn_extension = "GL_AMD_shader_stencil_export";
>>>>>>>     }
>>>>>>> +
>>>>>>> +   if (state->ARB_sample_shading_enable) {
>>>>>>> +      add_input(VARYING_SLOT_SAMPLE_ID, int_t, "gl_SampleID");
>>>>>>> +      add_input(VARYING_SLOT_SAMPLE_POS, vec2_t, "gl_SamplePosition");
>>>>>>> +      add_output(FRAG_RESULT_SAMPLE_MASK, array(int_t, 1), "gl_SampleMask");
>>>>>>
>>>>>> I don't see code anywhere in this patch series that correctly sizes this
>>>>>> array.  I thought gl_SampleMask.length() == gl_NumSamples... except you
>>>>>> can't use .length() on gl_SampleMask because the size can't be known at
>>>>>> compile-time.  We should have a test that 'gl_SampleMask.length()' fails
>>>>>> to compile. :)
>>>>>>
>>>>> I'll add a piglit compiler test for that.
>>>>>
>>>>>> The only other array that works like this is gl_TexCoord.  That array is
>>>>>> sized to 0 (line 837 of builtin_variables.cpp).  I believe gl_SampleMask
>>>>>> should do the same.
>>>>>>
>>>>> Yes, I noticed  gl_TexCoord is using array size of 0 and it works for
>>>>> gl_SampleMask as well.  But, It generates same glsl ir with array size
>>>>> of 0 or 1:
>>>>> (declare (shader_out ) (array int 1) gl_SampleMask)
>>>>>
>>>>> what's the utility of using size 0?
>>>>> I'll take care of rest of your comments in this patch.
>>>>
>>>> Using size 0 is like writing
>>>>
>>>> int gl_SampleMask[]; // unsized
>>>>
>>>> in the shader.  Later during compilation, usually in linking, unsized
>>>> arrays become sized.  Somewhere between generating the built-in variable
>>>> and printing the IR a size is established for it.
>>>
>>> Right, but gl_SampleMask is not an unsized array.  It's statically sized
>>> based on your implementation's maximum supported number of color samples
>>> (ceil(samples/32)), and this is totally known at compile time...
>>
>> That doesn't match what the spec says.  At least the GLSL 4.00 spec:
>>
>>     "In the fragment language, built-in variables are intrinsically
>>     declared as:
>>
>>         in vec4 gl_FragCoord;
>>         in bool gl_FrontFacing;
>>         in float gl_ClipDistance[];
>>         in vec2 gl_PointCoord;
>>         in int gl_PrimitiveID;
>>         in int gl_SampleID;
>>         in vec2 gl_SamplePosition;
>>         out vec4 gl_FragColor;
>>         out vec4 gl_FragData[gl_MaxDrawBuffers];
>>         out float gl_FragDepth;
>>         out int gl_SampleMask[];"
>>
>> Note that both gl_ClipDistance and gl_SampleMask are unsized, and both
>> have a maximum size based on implementation limits.  Later the GLSL 4.00
>> spec says:
>>
>>     "This array must be sized in the fragment shader either implicitly
>>     or explicitly to be the same size as the implementation-dependent
>>     maximum sample-mask (as an array of 32bit elements), determined by
>>     the maximum number of samples."
> 
> But the 4.40 spec fixed the madness:
> 
>     "For both the input array gl_SampleMaskIn[] and the output array
>     gl_SampleMask[], bit B of mask M (gl_SampleMaskIn[M] or
>     gl_SampleMask[M]) corresponds to sample 32*M+B. These arrays
>     have ceil(s/32) elements, where s is the maximum number of color
>     samples supported by the implementation."
> 
> Setting the array size to 1 is correct after all.  Ugh.

Gak.  And the "This array must be sized in the fragment shader either
implicitly or explicitly to be the same size described above." language
is still there.  Let's just size it to 1 since that seems to match
NVIDIA's behavior and be sensible.

>> The maximum size of the array is ceil(max_samples/32), but initially it
>> is unsized.
>>
>>> --Ken