diff --git a/tests/libprojectM/CMakeLists.txt b/tests/libprojectM/CMakeLists.txt index 4499d0d0fd..e914e996aa 100644 --- a/tests/libprojectM/CMakeLists.txt +++ b/tests/libprojectM/CMakeLists.txt @@ -28,6 +28,7 @@ test_api_headers(TestMainAPIHeaders add_executable(projectM-unittest HLSLParserTest.cpp + HLSLModuloTest.cpp LoggingTest.cpp MilkdropShaderCommentParsingTest.cpp PresetFileParserTest.cpp diff --git a/tests/libprojectM/HLSLModuloTest.cpp b/tests/libprojectM/HLSLModuloTest.cpp new file mode 100644 index 0000000000..05a7d23000 --- /dev/null +++ b/tests/libprojectM/HLSLModuloTest.cpp @@ -0,0 +1,166 @@ +#include + +#include +#include +#include + +#include +#include + +namespace { + +class ModuloTest : public testing::Test +{ +protected: + M4::Allocator allocator; + M4::HLSLTree tree{&allocator}; + M4::HLSLParser parser{&allocator, &tree}; + + bool Parse(const char* source) + { + return parser.Parse("modulo.hlsl", source, std::strlen(source)); + } + + M4::HLSLBinaryExpression* Result() + { + auto* declaration = tree.FindGlobalDeclaration("result"); + if (!declaration || !declaration->assignment || + declaration->assignment->nodeType != M4::HLSLNodeType_BinaryExpression) + return nullptr; + return static_cast(declaration->assignment); + } + + std::string Generate(M4::GLSLGenerator::Version version) + { + M4::GLSLGenerator generator; + if (!generator.Generate(&tree, M4::GLSLGenerator::Target_FragmentShader, version, "PS")) + return {}; + return generator.GetResult(); + } +}; + +TEST_F(ModuloTest, FloatVectorResultPreservesComponents) +{ + ASSERT_TRUE(Parse("float3 one; float3 two; float3 result = one % two;")); + ASSERT_NE(Result(), nullptr); + EXPECT_EQ(Result()->expressionType.baseType, M4::HLSLBaseType_Float3); +} + +TEST_F(ModuloTest, MixedScalarAndVectorPromotesToFloatVector) +{ + ASSERT_TRUE(Parse("float3 one; int two; float3 result = one % two;")); + ASSERT_NE(Result(), nullptr); + EXPECT_EQ(Result()->expressionType.baseType, M4::HLSLBaseType_Float3); +} + +TEST_F(ModuloTest, UnsignedVectorResultPreservesComponents) +{ + ASSERT_TRUE(Parse("uint3 one; uint3 two; uint3 result = one % two;")); + ASSERT_NE(Result(), nullptr); + EXPECT_EQ(Result()->expressionType.baseType, M4::HLSLBaseType_Uint3); +} + +TEST_F(ModuloTest, RemainderAndMultiplicationAssociateLeftToRight) +{ + ASSERT_TRUE(Parse("float3 one; float3 two; float3 result = one % two * 2;")); + ASSERT_NE(Result(), nullptr); + EXPECT_EQ(Result()->binaryOp, M4::HLSLBinaryOp_Mul); + ASSERT_EQ(Result()->expression1->nodeType, M4::HLSLNodeType_BinaryExpression); + EXPECT_EQ(static_cast(Result()->expression1)->binaryOp, + M4::HLSLBinaryOp_Mod); +} + +TEST_F(ModuloTest, ComparisonBindsMoreTightlyThanEquality) +{ + ASSERT_TRUE(Parse("int a; int b; int c; bool result = a < b == c;")); + ASSERT_NE(Result(), nullptr); + EXPECT_EQ(Result()->binaryOp, M4::HLSLBinaryOp_Equal); + ASSERT_EQ(Result()->expression1->nodeType, M4::HLSLNodeType_BinaryExpression); + EXPECT_EQ(static_cast(Result()->expression1)->binaryOp, + M4::HLSLBinaryOp_Less); +} + +TEST_F(ModuloTest, BitwiseAndBindsMoreTightlyThanXor) +{ + ASSERT_TRUE(Parse("int a; int b; int c; int result = a ^ b & c;")); + ASSERT_NE(Result(), nullptr); + EXPECT_EQ(Result()->binaryOp, M4::HLSLBinaryOp_BitXor); + ASSERT_EQ(Result()->expression2->nodeType, M4::HLSLNodeType_BinaryExpression); + EXPECT_EQ(static_cast(Result()->expression2)->binaryOp, + M4::HLSLBinaryOp_BitAnd); +} + +TEST_F(ModuloTest, FloatRemainderUsesModOnDesktopAndES) +{ + ASSERT_TRUE(Parse("float4 PS(float4 position : TEXCOORD0) : COLOR0 { " + "float3 one = position.xyz; float3 two = float3(0.3, 0.4, 0.5); " + "return float4(one % two * 2, 1); }")); + for (auto version : {M4::GLSLGenerator::Version_330, M4::GLSLGenerator::Version_300_ES, + M4::GLSLGenerator::Version_120}) + { + auto glsl = Generate(version); + ASSERT_FALSE(glsl.empty()); + EXPECT_NE(glsl.find("mod(one, two)"), std::string::npos); + EXPECT_EQ(glsl.find("int (one)"), std::string::npos); + } +} + +TEST_F(ModuloTest, IntegerVectorRemainderKeepsIntegerOperator) +{ + ASSERT_TRUE(Parse("float4 PS(float4 position : TEXCOORD0) : COLOR0 { " + "int3 one = int3(5, 7, 9); int3 two = int3(2, 3, 4); " + "return float4(one % two, 1); }")); + auto glsl = Generate(M4::GLSLGenerator::Version_330); + ASSERT_FALSE(glsl.empty()); + EXPECT_NE(glsl.find("one % two"), std::string::npos); + EXPECT_EQ(glsl.find("int (one)"), std::string::npos); +} + +TEST_F(ModuloTest, IntegerVectorRemainderUsesVectorFallbackOnLegacyGLSL) +{ + ASSERT_TRUE(Parse("float4 PS(float4 position : TEXCOORD0) : COLOR0 { " + "int3 one = int3(5, 7, 9); int3 two = int3(2, 3, 4); " + "return float4(one % two, 1); }")); + auto glsl = Generate(M4::GLSLGenerator::Version_120); + ASSERT_FALSE(glsl.empty()); + EXPECT_NE(glsl.find("ivec3(mod("), std::string::npos); + EXPECT_EQ(glsl.find("int (one)"), std::string::npos); +} + +TEST_F(ModuloTest, GeneratedRemainderPromotesScalarAndVectorOperands) +{ + struct Case { const char* declarations; const char* operation; bool floating; }; + for (const auto& test : { + Case{"float one = 0.75; float two = 0.5;", "one % two", true}, + Case{"float3 one = float3(0.75, 0.5, 0.25); int two = 2;", "one % two", true}, + Case{"int one = 2; float3 two = float3(0.75, 0.5, 0.25);", "one % two", true}, + Case{"int3 one = int3(5, 7, 9); int two = 2;", "one % two", false}, + Case{"uint one = 5; uint two = 2;", "one % two", false}, + Case{"uint3 one = uint3(5, 7, 9); uint two = 2;", "one % two", false}}) + { + SCOPED_TRACE(test.declarations); + M4::Allocator localAllocator; + M4::HLSLTree localTree(&localAllocator); + M4::HLSLParser localParser(&localAllocator, &localTree); + std::string source = "float4 PS(float4 position : TEXCOORD0) : COLOR0 { "; + source += test.declarations; + source += " float3 result = "; + source += test.operation; + source += "; return float4(result, 1); }"; + ASSERT_TRUE(localParser.Parse("mixed-modulo.hlsl", source.c_str(), source.size())); + for (auto version : {M4::GLSLGenerator::Version_330, M4::GLSLGenerator::Version_300_ES}) + { + M4::GLSLGenerator generator; + ASSERT_TRUE(generator.Generate(&localTree, M4::GLSLGenerator::Target_FragmentShader, + version, "PS")); + std::string glsl = generator.GetResult(); + if (test.floating) + EXPECT_NE(glsl.find("mod("), std::string::npos); + else + EXPECT_NE(glsl.find(" % "), std::string::npos); + EXPECT_EQ(glsl.find("int (one)"), std::string::npos); + } + } +} + +} // namespace diff --git a/vendor/hlslparser/src/GLSLGenerator.cpp b/vendor/hlslparser/src/GLSLGenerator.cpp index c913e82682..dbe0c1349f 100644 --- a/vendor/hlslparser/src/GLSLGenerator.cpp +++ b/vendor/hlslparser/src/GLSLGenerator.cpp @@ -708,7 +708,18 @@ void GLSLGenerator::OutputExpression(HLSLExpression* expression, const HLSLType* // bool vectorExpression = IsVectorType( binaryExpression->expression1->expressionType ) || IsVectorType( binaryExpression->expression2->expressionType ); - if( vectorExpression && IsCompareOp( binaryExpression->binaryOp )) + if (binaryExpression->binaryOp == HLSLBinaryOp_Mod && + baseTypeDescriptions[binaryExpression->expressionType.baseType].numericType == NumericType_Float) + { + // HLSL permits floating-point remainder; GLSL's % only accepts integers. + // mod agrees with HLSL % in its defined domain (operands of the same sign). + m_writer.Write("mod("); + OutputExpression(binaryExpression->expression1, &binaryExpression->expressionType); + m_writer.Write(", "); + OutputExpression(binaryExpression->expression2, &binaryExpression->expressionType); + m_writer.Write(")"); + } + else if( vectorExpression && IsCompareOp( binaryExpression->binaryOp )) { switch (binaryExpression->binaryOp) { @@ -741,7 +752,7 @@ void GLSLGenerator::OutputExpression(HLSLExpression* expression, const HLSLType* case HLSLBinaryOp_Sub: op = " - "; dstType1 = dstType2 = &binaryExpression->expressionType; break; case HLSLBinaryOp_Mul: op = " * "; dstType1 = dstType2 = &binaryExpression->expressionType; break; case HLSLBinaryOp_Div: op = " / "; dstType1 = dstType2 = &binaryExpression->expressionType; break; - case HLSLBinaryOp_Mod: op = " % "; dstType1 = dstType2 = &kIntType; break; + case HLSLBinaryOp_Mod: op = " % "; dstType1 = dstType2 = &binaryExpression->expressionType; break; case HLSLBinaryOp_Less: op = " < "; dstType1 = dstType2 = commonScalarType(binaryExpression->expression1->expressionType, binaryExpression->expression2->expressionType); break; case HLSLBinaryOp_Greater: op = " > "; dstType1 = dstType2 = commonScalarType(binaryExpression->expression1->expressionType, binaryExpression->expression2->expressionType); break; case HLSLBinaryOp_LessEqual: op = " <= "; dstType1 = dstType2 = commonScalarType(binaryExpression->expression1->expressionType, binaryExpression->expression2->expressionType); break; @@ -762,11 +773,14 @@ void GLSLGenerator::OutputExpression(HLSLExpression* expression, const HLSLType* ASSERT(0); } if ((m_version == Version_110 || m_version == Version_120 || m_version == Version_100_ES) && binaryExpression->binaryOp == HLSLBinaryOp_Mod) { - m_writer.Write("(int(mod("); - OutputExpression(binaryExpression->expression1, dstType1); - m_writer.Write(","); - OutputExpression(binaryExpression->expression2, dstType2); - m_writer.Write(")))"); + // Legacy GLSL has no integer %, so use a component-wise float fallback. + HLSLType floatType(static_cast(HLSLBaseType_Float + + baseTypeDescriptions[binaryExpression->expressionType.baseType].numComponents - 1)); + m_writer.Write("%s(mod(", GetTypeName(binaryExpression->expressionType)); + OutputExpression(binaryExpression->expression1, &floatType); + m_writer.Write(", "); + OutputExpression(binaryExpression->expression2, &floatType); + m_writer.Write("))"); } else { bool handled = false; if (m_options.flags & Flag_AlternateNanPropagation) { diff --git a/vendor/hlslparser/src/HLSLParser.cpp b/vendor/hlslparser/src/HLSLParser.cpp index 03ba4d92a6..43ad3db511 100644 --- a/vendor/hlslparser/src/HLSLParser.cpp +++ b/vendor/hlslparser/src/HLSLParser.cpp @@ -712,13 +712,15 @@ const int _binaryOpPriority[] = { 2, 1, // &&, || 8, 8, // +, - - 9, 9, // *, / + 9, 9, 9, // *, /, % 7, 7, // <, >, 7, 7, // <=, >=, 6, 6, // ==, != 5, 3, 4, // &, |, ^ }; +static_assert(sizeof(_binaryOpPriority) / sizeof(_binaryOpPriority[0]) == HLSLBinaryOp_Assign, + "Binary operator priorities must match HLSLBinaryOp"); // IC: I'm not sure this table is right, but any errors should be caught by the backend compiler. // Also, this is operator dependent. The type resulting from (float4 * float4x4) is not the same as (float4 + float4x4). @@ -1234,10 +1236,6 @@ static bool GetBinaryOpResultType(HLSLBinaryOp binaryOp, const HLSLType& type1, result.baseType = HLSLBaseType( HLSLBaseType_Bool + numComponents - 1 ); break; } - case HLSLBinaryOp_Mod: - result.baseType = HLSLBaseType_Int; - break; - default: result.baseType = _binaryOpTypeLookup[type1.baseType - HLSLBaseType_FirstNumeric][type2.baseType - HLSLBaseType_FirstNumeric]; break;