From 06586e05bf8d533b57646aa983479e034a3176f2 Mon Sep 17 00:00:00 2001 From: Piotr Byszewski Date: Tue, 30 Jan 2018 15:19:34 +0100 Subject: [PATCH] Fix invalid_assign_to_1 fragdata tests For ES3 capable hardware compilation of shaders in invalid_assign_to_1 test succeeds resulting in test failure. This patch resolves that by adding only_glsl_es_100_support and exactly_one_draw_buffer flags that indicate that test shouldn't be executed on ES3 capable hardwere or on a implementation that supports more then one draw buffer. This change also removes redundant CaseRequirement structure and parses requirements directly to ShaderCaseSpecification. Components: OpenGL VK-GL-CTS issue: 282 Affects: dEQP-GLES2.functional.shaders.fragdata.invalid_assign_to_1 dEQP-GLES2.functional.shaders.* dEQP-GLES3.functional.shaders.fragdata.invalid_assign_to_1 dEQP-GLES3.functional.shaders.* Change-Id: I44949fde7ac9bc724965ec166ed55433c91857c2 --- android/cts/master/gles2-master.txt | 1 + android/cts/master/gles3-master.txt | 1 + android/cts/master/src/gles2-failures.txt | 1 - android/cts/master/src/gles3-driver-issues.txt | 1 - data/gles2/shaders/fragdata.test | 2 + data/gles3/shaders/fragdata.test | 2 + framework/opengl/gluShaderLibrary.cpp | 121 ++++++++----------------- framework/opengl/gluShaderLibrary.hpp | 39 ++++++-- modules/glshared/glsShaderLibraryCase.cpp | 30 +++++- 9 files changed, 102 insertions(+), 96 deletions(-) diff --git a/android/cts/master/gles2-master.txt b/android/cts/master/gles2-master.txt index b292fa5..855861b 100644 --- a/android/cts/master/gles2-master.txt +++ b/android/cts/master/gles2-master.txt @@ -6918,6 +6918,7 @@ dEQP-GLES2.functional.shaders.invariance.lowp.loop_2 dEQP-GLES2.functional.shaders.invariance.lowp.loop_3 dEQP-GLES2.functional.shaders.invariance.lowp.loop_4 dEQP-GLES2.functional.shaders.fragdata.valid_static_index +dEQP-GLES2.functional.shaders.fragdata.invalid_assign_to_1 dEQP-GLES2.functional.shaders.fragdata.write_fragcolor_and_fragdata_simple dEQP-GLES2.functional.shaders.algorithm.hsl_to_rgb_vertex dEQP-GLES2.functional.shaders.algorithm.hsl_to_rgb_fragment diff --git a/android/cts/master/gles3-master.txt b/android/cts/master/gles3-master.txt index 15ec145..b82fd74 100644 --- a/android/cts/master/gles3-master.txt +++ b/android/cts/master/gles3-master.txt @@ -18948,6 +18948,7 @@ dEQP-GLES3.functional.shaders.invariance.lowp.loop_4 dEQP-GLES3.functional.shaders.fragdata.valid_static_index dEQP-GLES3.functional.shaders.fragdata.valid_uniform_index dEQP-GLES3.functional.shaders.fragdata.valid_dynamic_index +dEQP-GLES3.functional.shaders.fragdata.invalid_assign_to_1 dEQP-GLES3.functional.shaders.fragdata.write_fragcolor_and_fragdata_simple dEQP-GLES3.functional.shaders.fragdata.write_fragcolor_and_fragdata_static_if dEQP-GLES3.functional.shaders.fragdata.write_fragcolor_and_fragdata_unused_func diff --git a/android/cts/master/src/gles2-failures.txt b/android/cts/master/src/gles2-failures.txt index 3d4e7c6..e31a97c 100644 --- a/android/cts/master/src/gles2-failures.txt +++ b/android/cts/master/src/gles2-failures.txt @@ -1239,7 +1239,6 @@ dEQP-GLES2.functional.shaders.declarations.invalid_declarations.invariant_unifor dEQP-GLES2.functional.shaders.discard.dynamic_loop_texture dEQP-GLES2.functional.shaders.discard.function_static_loop_texture dEQP-GLES2.functional.shaders.discard.static_loop_texture -dEQP-GLES2.functional.shaders.fragdata.invalid_assign_to_1 dEQP-GLES2.functional.shaders.fragdata.valid_dynamic_index dEQP-GLES2.functional.shaders.fragdata.valid_uniform_index dEQP-GLES2.functional.shaders.fragdata.write_fragcolor_and_fragdata_static_if diff --git a/android/cts/master/src/gles3-driver-issues.txt b/android/cts/master/src/gles3-driver-issues.txt index 99c22e6..baf0d71 100644 --- a/android/cts/master/src/gles3-driver-issues.txt +++ b/android/cts/master/src/gles3-driver-issues.txt @@ -7,7 +7,6 @@ dEQP-GLES3.functional.shaders.preprocessor.predefined_macros.line_2_fragment # Bug 22488431 dEQP-GLES3.functional.shaders.fragdata.draw_buffers -dEQP-GLES3.functional.shaders.fragdata.invalid_assign_to_1 # Bug 22632106 dEQP-GLES3.functional.shaders.texture_functions.texturelodoffset.usampler2darray_vertex diff --git a/data/gles2/shaders/fragdata.test b/data/gles2/shaders/fragdata.test index f7ba4f2..63af9eb 100644 --- a/data/gles2/shaders/fragdata.test +++ b/data/gles2/shaders/fragdata.test @@ -2,6 +2,8 @@ case invalid_assign_to_1 version 100 es expect compile_fail + require only_glsl_es_100_support + require exactly_one_draw_buffer vertex "" void main (void) { diff --git a/data/gles3/shaders/fragdata.test b/data/gles3/shaders/fragdata.test index f7ba4f2..63af9eb 100644 --- a/data/gles3/shaders/fragdata.test +++ b/data/gles3/shaders/fragdata.test @@ -2,6 +2,8 @@ case invalid_assign_to_1 version 100 es expect compile_fail + require only_glsl_es_100_support + require exactly_one_draw_buffer vertex "" void main (void) { diff --git a/framework/opengl/gluShaderLibrary.cpp b/framework/opengl/gluShaderLibrary.cpp index 892c88b..8343cba 100644 --- a/framework/opengl/gluShaderLibrary.cpp +++ b/framework/opengl/gluShaderLibrary.cpp @@ -105,7 +105,7 @@ bool isValid (const ShaderCaseSpecification& spec) return false; } - if (spec.fullGLSLES100Required) + if (isCapabilityRequired(CAPABILITY_FULL_GLSL_ES_100_SUPPORT, spec)) { if (spec.targetVersion != GLSL_VERSION_100_ES) { @@ -233,6 +233,19 @@ bool isValid (const ShaderCaseSpecification& spec) return true; } +bool isCapabilityRequired(CapabilityFlag capabilityFlag, const ShaderCaseSpecification& spec) +{ + std::vector::const_iterator currRequirement = spec.requiredCaps.begin(); + while (currRequirement != spec.requiredCaps.end()) + { + if ((currRequirement->type == CAPABILITY_FLAG) && (currRequirement->flagName == capabilityFlag)) + return true; + ++currRequirement; + } + + return false; +} + // Parser static const glu::GLSLVersion DEFAULT_GLSL_VERSION = glu::GLSL_VERSION_100_ES; @@ -262,51 +275,6 @@ DE_INLINE deBool isCaseNameChar (char c) return deInRange32(c, 'a', 'z') || deInRange32(c, 'A', 'Z') || deInRange32(c, '0', '9') || (c == '_') || (c == '-') || (c == '.'); } -struct CaseRequirement -{ - enum Type - { - TYPE_EXTENSION = 0, - TYPE_FULL_GLSL_ES_100_SUPPORT, - TYPE_IMPLEMENTATION_LIMIT, - - TYPE_LAST - }; - - Type type; - - // TYPE_EXTENSION: - RequiredExtension extension; - - // TYPE_IMPLEMENTATION_LIMIT - RequiredCapability requiredCap; - - CaseRequirement (void) : type(TYPE_LAST) {} - - static CaseRequirement createFullGLSLES100SpecificationRequirement (void) - { - CaseRequirement req; - req.type = TYPE_FULL_GLSL_ES_100_SUPPORT; - return req; - } - - static CaseRequirement createAnyExtensionRequirement (const vector& alternatives, deUint32 effectiveStages) - { - CaseRequirement req; - req.type = TYPE_EXTENSION; - req.extension = RequiredExtension(alternatives, effectiveStages); - return req; - } - - static CaseRequirement createLimitRequirement (deUint32 enumName, int referenceValue) - { - CaseRequirement req; - req.type = TYPE_IMPLEMENTATION_LIMIT; - req.requiredCap = RequiredCapability(enumName, referenceValue); - return req; - } -}; - class ShaderParser { public: @@ -414,7 +382,7 @@ private: void parseValue (ValueBlock& valueBlock); void parseValueBlock (ValueBlock& valueBlock); deUint32 parseShaderStageList (void); - void parseRequirement (CaseRequirement& valueBlock); + void parseRequirement (vector &requiredCaps, vector &requiredExts); void parseExpectResult (ExpectResult& expectResult); void parseFormat (DataType& format); void parseGLSLVersion (glu::GLSLVersion& version); @@ -1152,7 +1120,7 @@ deUint32 ShaderParser::parseShaderStageList (void) return mask; } -void ShaderParser::parseRequirement (CaseRequirement& valueBlock) +void ShaderParser::parseRequirement (vector& requiredCaps, vector& requiredExts) { PARSE_DBG((" parseRequirement()\n")); @@ -1196,7 +1164,7 @@ void ShaderParser::parseRequirement (CaseRequirement& valueBlock) affectedCasesFlags = parseShaderStageList(); } - valueBlock = CaseRequirement::createAnyExtensionRequirement(anyExtensionStringList, affectedCasesFlags); + requiredExts.push_back(RequiredExtension(anyExtensionStringList, affectedCasesFlags)); } else if (m_curTokenStr == "limit") { @@ -1216,13 +1184,25 @@ void ShaderParser::parseRequirement (CaseRequirement& valueBlock) limitValue = parseIntLiteral(m_curTokenStr.c_str()); advanceToken(); - valueBlock = CaseRequirement::createLimitRequirement(limitEnum, limitValue); + requiredCaps.push_back(RequiredCapability(limitEnum, limitValue)); } else if (m_curTokenStr == "full_glsl_es_100_support") { advanceToken(); - valueBlock = CaseRequirement::createFullGLSLES100SpecificationRequirement(); + requiredCaps.push_back(RequiredCapability(CAPABILITY_FULL_GLSL_ES_100_SUPPORT)); + } + else if (m_curTokenStr == "only_glsl_es_100_support") + { + advanceToken(); + + requiredCaps.push_back(RequiredCapability(CAPABILITY_ONLY_GLSL_ES_100_SUPPORT)); + } + else if (m_curTokenStr == "exactly_one_draw_buffer") + { + advanceToken(); + + requiredCaps.push_back(RequiredCapability(CAPABILITY_EXACTLY_ONE_DRAW_BUFFER)); } else parseError(string("invalid requirement value: " + m_curTokenStr)); @@ -1307,12 +1287,11 @@ void ShaderParser::parsePipelineProgram (ProgramSpecification& program) } else if (m_curToken == TOKEN_REQUIRE) { - CaseRequirement requirement; - parseRequirement(requirement); + vector dummyCaps; + size_t size = program.requiredExtensions.size(); + parseRequirement(dummyCaps, program.requiredExtensions); - if (requirement.type == CaseRequirement::TYPE_EXTENSION) - program.requiredExtensions.push_back(requirement.extension); - else + if (size == program.requiredExtensions.size()) parseError("only extension requirements are allowed inside pipeline program"); } else if (m_curToken == TOKEN_VERTEX || @@ -1375,7 +1354,8 @@ void ShaderParser::parseShaderCase (vector& shaderNodeList) vector geometrySources; ValueBlock valueBlock; bool valueBlockSeen = false; - vector requirements; + vector requiredCaps; + vector requiredExts; vector pipelinePrograms; for (;;) @@ -1448,9 +1428,7 @@ void ShaderParser::parseShaderCase (vector& shaderNodeList) } else if (m_curToken == TOKEN_REQUIRE) { - CaseRequirement requirement; - parseRequirement(requirement); - requirements.push_back(requirement); + parseRequirement(requiredCaps, requiredExts); } else if (m_curToken == TOKEN_PIPELINE_PROGRAM) { @@ -1465,25 +1443,6 @@ void ShaderParser::parseShaderCase (vector& shaderNodeList) advanceToken(TOKEN_END); // case end - // \todo [pyry] Clean up - vector requiredCaps; - vector requiredExts; - bool fullGLSLES100Required = false; - - for (size_t reqNdx = 0; reqNdx < requirements.size(); ++reqNdx) - { - const CaseRequirement& requirement = requirements[reqNdx]; - - if (requirement.type == CaseRequirement::TYPE_EXTENSION) - requiredExts.push_back(requirement.extension); - else if (requirement.type == CaseRequirement::TYPE_IMPLEMENTATION_LIMIT) - requiredCaps.push_back(requirement.requiredCap); - else if (requirement.type == CaseRequirement::TYPE_FULL_GLSL_ES_100_SUPPORT) - fullGLSLES100Required = true; - else - DE_ASSERT(false); - } - if (!bothSource.empty()) { if (!vertexSources.empty() || @@ -1502,7 +1461,6 @@ void ShaderParser::parseShaderCase (vector& shaderNodeList) spec.caseType = CASETYPE_VERTEX_ONLY; spec.expectResult = expectResult; spec.targetVersion = version; - spec.fullGLSLES100Required = fullGLSLES100Required; spec.requiredCaps = requiredCaps; spec.values = valueBlock; @@ -1519,7 +1477,6 @@ void ShaderParser::parseShaderCase (vector& shaderNodeList) spec.caseType = CASETYPE_FRAGMENT_ONLY; spec.expectResult = expectResult; spec.targetVersion = version; - spec.fullGLSLES100Required = fullGLSLES100Required; spec.requiredCaps = requiredCaps; spec.values = valueBlock; @@ -1538,7 +1495,6 @@ void ShaderParser::parseShaderCase (vector& shaderNodeList) spec.outputType = outputType; spec.outputFormat = format; spec.targetVersion = version; - spec.fullGLSLES100Required = fullGLSLES100Required; spec.requiredCaps = requiredCaps; spec.values = valueBlock; @@ -1572,7 +1528,6 @@ void ShaderParser::parseShaderCase (vector& shaderNodeList) spec.caseType = CASETYPE_COMPLETE; spec.expectResult = expectResult; spec.targetVersion = version; - spec.fullGLSLES100Required = fullGLSLES100Required; spec.requiredCaps = requiredCaps; spec.values = valueBlock; diff --git a/framework/opengl/gluShaderLibrary.hpp b/framework/opengl/gluShaderLibrary.hpp index f89aec1..5e8b1d1 100644 --- a/framework/opengl/gluShaderLibrary.hpp +++ b/framework/opengl/gluShaderLibrary.hpp @@ -86,19 +86,43 @@ struct ValueBlock std::vector uniforms; }; +enum CapabilityType +{ + CAPABILITY_LIMIT = 0, + CAPABILITY_FLAG, + + CAPABILITY_LAST +}; + +enum CapabilityFlag +{ + CAPABILITY_FULL_GLSL_ES_100_SUPPORT, + CAPABILITY_ONLY_GLSL_ES_100_SUPPORT, // only ES2, no ES3 capability + CAPABILITY_EXACTLY_ONE_DRAW_BUFFER // gl_MaxDrawBuffers is exactly 1 +}; + struct RequiredCapability { - deUint32 enumName; + CapabilityType type; + + union + { + CapabilityFlag flagName; + deUint32 enumName; + }; + int referenceValue; - RequiredCapability (void) - : enumName (0u) - , referenceValue (0) + RequiredCapability (CapabilityFlag flagName_) + : type (CAPABILITY_FLAG) + , flagName (flagName_) + , referenceValue (0) // not used { } RequiredCapability (deUint32 enumName_, int referenceValue_) - : enumName (enumName_) + : type (CAPABILITY_LIMIT) + , enumName (enumName_) , referenceValue (referenceValue_) { } @@ -149,9 +173,7 @@ struct ShaderCaseSpecification DataType outputFormat; glu::GLSLVersion targetVersion; - // \todo [pyry] Clean this up std::vector requiredCaps; - bool fullGLSLES100Required; ValueBlock values; std::vector programs; @@ -162,7 +184,6 @@ struct ShaderCaseSpecification , outputType (OUTPUT_RESULT) , outputFormat (TYPE_LAST) , targetVersion (glu::GLSL_VERSION_LAST) - , fullGLSLES100Required (false) { } }; @@ -170,6 +191,8 @@ struct ShaderCaseSpecification bool isValid (const ValueBlock& block); bool isValid (const ShaderCaseSpecification& spec); +bool isCapabilityRequired(CapabilityFlag capabilityFlag, const ShaderCaseSpecification& spec); + class ShaderCaseFactory { public: diff --git a/modules/glshared/glsShaderLibraryCase.cpp b/modules/glshared/glsShaderLibraryCase.cpp index 170dcfe..1a40998 100644 --- a/modules/glshared/glsShaderLibraryCase.cpp +++ b/modules/glshared/glsShaderLibraryCase.cpp @@ -119,8 +119,12 @@ static void checkImplementationLimits (const vector& require { for (size_t capNdx = 0; capNdx < requiredCaps.size(); ++capNdx) { - const deUint32 pname = requiredCaps[capNdx].enumName; - const int requiredValue = requiredCaps[capNdx].referenceValue; + const RequiredCapability& capability = requiredCaps[capNdx]; + if (capability.type != CAPABILITY_LIMIT) + continue; + + const deUint32 pname = capability.enumName; + const int requiredValue = capability.referenceValue; const int supportedValue = ctxInfo.getInt((int)pname); if (supportedValue <= requiredValue) @@ -941,6 +945,26 @@ bool ShaderLibraryCase::execute (void) GLU_EXPECT_NO_ERROR(gl.getError(), "ShaderCase::execute(): start"); + if(isCapabilityRequired(CAPABILITY_ONLY_GLSL_ES_100_SUPPORT, m_spec)) + { + // GL_MAJOR_VERSION query does not exist on GLES2 + // so succeeding query implies GLES3+ hardware. + glw::GLint majorVersion = 0; + gl.getIntegerv(GL_MAJOR_VERSION, &majorVersion); + if (gl.getError() == GL_NO_ERROR) + return true; + } + + if(isCapabilityRequired(CAPABILITY_EXACTLY_ONE_DRAW_BUFFER, m_spec)) + { + // on unextended ES2 there is only one draw buffer + // and there is no GL_MAX_DRAW_BUFFERS query + glw::GLint maxDrawBuffers = 0; + gl.getIntegerv(GL_MAX_DRAW_BUFFERS, &maxDrawBuffers); + if ((gl.getError() == GL_NO_ERROR) && (maxDrawBuffers > 1)) + throw tcu::NotSupportedError("Test requires exactly one draw buffer"); + } + // Specialize shaders if (m_spec.caseType == CASETYPE_VERTEX_ONLY) { @@ -1080,7 +1104,7 @@ bool ShaderLibraryCase::execute (void) // \todo [2010-06-07 petri] These should be handled in the test case? log << TestLog::Message << "ERROR: " << failReason << TestLog::EndMessage; - if (m_spec.fullGLSLES100Required) + if (isCapabilityRequired(CAPABILITY_FULL_GLSL_ES_100_SUPPORT, m_spec)) { log << TestLog::Message << "Assuming build failure is caused by implementation not supporting full GLSL ES 100 specification, which is not required." -- 2.7.4