On 10/16/2013 11:29 AM, Kenneth Graunke wrote:
> On 10/16/2013 10:40 AM, Ian Romanick wrote:
>> On 10/15/2013 07:50 PM, Kenneth Graunke wrote:
> [snip]
>>> 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."
>>
>> The maximum size of the array is ceil(max_samples/32), but initially it
>> is unsized.
>
> You've got to be kidding me. This is utter and complete garbage.
> "We know exactly how big the array is, at compile time, but we're going
> to make you, the technical artist, tell us anyway. By the way, you have
> to guess the exact number we're thinking."
>
> So, about that "explicitly the same size" text...what if my shader sets
> it to some /other/ size? What happens?
>
> * Larger than ceil(float(gl_NumSamples)/32.0):
>
> A compile error? Truncated to the right size? Probably the same
> behavior as ClipDistance. That doesn't seem well specified either.
>
> * Smaller than ceil(float(gl_NumSamples)/32.0):
>
> From the GLSL 4.40 specification,
> "If the fragment shader statically assigns a value to gl_SampleMask,
> the sample mask will be undefined for any array elements of any
> fragment shader invocations that fail to assign a value."
>
> The hardware is expecting a certain number of samples, so ultimately,
> the fact that you don't have them means they're undefined. Just like
> the above case of forgetting to assign them. Do we pretend the array is
> the right size, ignoring their request and treating any unwritten
> elements as unsized? Or do we reject it as a compile error?
>
> ...
>
> Also, considering that the Radeon 8970 (arguably AMD's best card today)
> only does 24x MSAA, and it looks like nVidia's GTX Titan doesn't do more
> than 32x (though it's harder to tell), this array will always be exactly
> size 1 for every implementation in existence.
Looking that revision history, it was a fairly late change to make
gl_SampleMask an array at all. It was just a scalar int.
> <sarcasm>
> I'm sure applications are fully on board to explicitly declare this and
> cope with the consequences of it not being a single integer.
> </sarcasm>
Maybe. I doubt there are very many applications that use gl_SampleMask.
Of the ones that do, they probably also use gl_SampleMaskIn and do
something like:
gl_SampleMask = gl_SampleMaskIn; // of course this fails. duh
or
for (...)
gl_SampleMask[i] = gl_SampleMaskIn[i];
Note that the spec says "implicitly or explicitly". The latter form
implicitly sizes gl_SampleMask to be the same size as gl_SampleMaskIn.
In ARB_gpu_shader5, gl_SampleMaskIn has somewhat more sensible language:
in int gl_SampleMaskIn[];
The variable gl_SampleMaskIn is an array of integers, each holding a
bitfield indicating the set of samples covered by the primitive
generating the fragment during multisample rasterization. The
array has ceil(<s>/32) elements, where <s> is the maximum number of
color samples supported by the implementation. Bit <n> or word <w>
in the bitfield is set if and only if the sample numbered
<w>*32+<n> is considered covered for this fragment shader
invocation.
> That said, at this point, it's about 4 years too late to change the
> idiocy of the spec, so we're kind of stuck. We probably ought to
> investigate what other vendors do.
You are completely correct here. We should check what other vendors do.
I think 5 tests will tell us everything we need to know (then we'll
probably submit some spec bugs). I don't have any non-Intel hardware
that supports either extension, so I can't actually try any of these.
1. Does this compile, link, and run:
#version 130
#extension GL_ARB_sample_shading: require
out vec4 color;
void main() {
color = vec4(gl_SampleMask.length());
gl_SampleMask[0] = ~0;
}
2. Does this compile and link:
#version 130
#extension GL_ARB_sample_shading: require
out vec4 color;
void main() {
color = vec4(1);
gl_SampleMask[0] = ~0;
gl_SampleMask[1] = ~0;
}
3. Does this compile and link:
#version 130
#extension GL_ARB_sample_shading: require
#extension GL_ARB_gpu_shader5: require
out vec4 color;
void main() {
color = vec4(1);
gl_SampleMask = gl_SampleMaskIn;
}
4. Does this compile and link:
#version 130
#extension GL_ARB_sample_shading: require
out vec4 color;
in int gl_SampleMask[1];
void main() {
color = vec4(1);
gl_SampleMask[0] = ~0;
}
5. Does this compile and link:
#version 130
#extension GL_ARB_sample_shading: require
out vec4 color;
in int gl_SampleMask[2];
void main() {
color = vec4(1);
gl_SampleMask[0] = ~0;
gl_SampleMask[1] = ~0;
}
> --Ken
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
