The array size of gl_SampleMask and gl_SampleMaskIn is ceil(gl_MaxSamples/32)

[jimihem]

#version 320 es

layout(location = 0) out mediump vec4 fragColor;
void main (void)
{
for (int i = 0; i < gl_SampleMask.length(); ++i)
gl_SampleMask[i] = int(0xAAAAAAAA);

   fragColor = vec4(0.0, 1.0, 0.0, 1.0);

}
gl_SampleMask.length() report error: “array must have size before use length"

dEQP-GLES31.functional.shaders.sample_variables.sample_mask.discard_half_per_pixel.default_framebuffer

[CLAassistant]

All committers have signed the CLA.

[ncesario-lunarg]

@jimihem Can you please add a test to validate this change?

It might also be helpful if the commit message is a bit more explicit with respect to what is being fixed.

[jimihem]

@ncesario-lunarg How to add a test to glslang?

[jimihem]

@ncesario-lunarg I have add a test to glslang, Can you help to merge this pr?

[ncesario-lunarg]

Thanks for these changes @jimihem, and sorry for the late reply (somehow I missed your previous comment).

This LGTM with just a few minor nits for the tests:

1. runtests is generally for testing the standalone glslang executable, but I don't think this change needs to test anything with respect to the standalone executable specifically? If that's true, then I would revert the changes to runtests and instead add the test file you added to gtests/AST.FromFile.cpp. e..g,:

diff --git a/gtests/AST.FromFile.cpp b/gtests/AST.FromFile.cpp
index cf56dae5..540cd99b 100644
--- a/gtests/AST.FromFile.cpp
+++ b/gtests/AST.FromFile.cpp
@@ -282,6 +282,7 @@ INSTANTIATE_TEST_SUITE_P(
         "glsl.es320.subgroupQuad.comp",
         "glsl.es320.subgroupVote.comp",
         "glsl.es320.extTextureShadowLod.frag",
+        "gl_samplemask_array_size.frag",
         "glsl.ext.textureShadowLod.frag",
         "terminate.frag",
         "terminate.vert",

Once you have that, you can run the gtest executable in "update mode" to automatically update the test results file (e.g., build/gtests/glslangtests --update-mode), though you should double check the result file contents afterward.

2. The result file should be named <test name>.out, where <test name> includes the extension, so you just need to rename Test/baseResults/gl_samplemask_array_size.out to Test/baseResults/gl_samplemask_array_size.frag.out. Running glslangtest in "update mode" may do this for you, but then you'll need to remove Test/baseResults/gl_samplemask_array_size.out.

[ncesario-lunarg]

One question, but otherwise LGTM modulo some changes to the tests.

@jimihem If you're ok with me updating your branch, I can push the testing changes I suggested.

[ncesario-lunarg]

Would it make more sense to instead get the qualifier and compare against EbvSampleMask here? e.g.,

} else if (const auto typed = intermNode->getAsTyped()) {
    if (typed->getQualifier().builtIn == EbvSampleMask) {

[jimihem]

@ncesario-lunarg Thank you for your reply， I have made some changes to this pr based on your suggestion.

[ncesario-lunarg]

Thanks @jimihem, this LGTM, but I'll give @arcady-lunarg a chance to take a look before merging.

[arcady-lunarg]

I think it might be a good idea to add a test for the case where gl_MaxSamples is greater than 32 (something like 64 would probably be a good value since that's the highest I have heard of).

[jimihem]

@arcady-lunarg gl_MaxSamples cann't be redeclared, when I try to redeclared it and initialize it as 64, glslang will report error.
glslangtests use default builtin resource and gl_MaxSamples is 4, so I cann't modify gl_MaxSamples to 64.

[arcady-lunarg]

@jimihem I think this would be a good case for a runtests style test, passing 2 different configurations to glslang and having 2 different expected results based on which configuration is used.

[arcady-lunarg]

Thanks, this looks good.