Workaround the unary minus operator issue on Intel

include/GLSLANG/ShaderLang.h

@@ -95,6 +95,7 @@ typedef enum
 } ShShaderOutput;
 
 // Compile options.
+
 typedef uint64_t ShCompileOptions;
 
 const ShCompileOptions SH_VALIDATE                           = 0;
@@ -199,6 +200,10 @@ const ShCompileOptions SH_REWRITE_TEXELFETCHOFFSET_TO_TEXELFETCH = UINT64_C(1) <
 // Condition calculation is not correct. Rewrite it from "CONDITION" to "CONDITION && true".
 const ShCompileOptions SH_ADD_AND_TRUE_TO_LOOP_CONDITION = UINT64_C(1) << 25;
 
+// This flag works around a bug in evaluating unary minus operator on integer on some INTEL
+// drivers. It works by translating -(int) into ~(int) + 1.
+const ShCompileOptions SH_REWRITE_INTEGER_UNARY_MINUS_OPERATOR = UINT64_C(1) << 26;
+
 // Defines alternate strategies for implementing array index clamping.
 typedef enum {
   // Use the clamp intrinsic for array index clamping.

src/compiler.gypi

@@ -98,6 +98,8 @@
             'compiler/translator/RewriteDoWhile.h',
             'compiler/translator/RewriteTexelFetchOffset.cpp',
             'compiler/translator/RewriteTexelFetchOffset.h',
+            'compiler/translator/RewriteUnaryMinusOperatorInt.cpp',
+            'compiler/translator/RewriteUnaryMinusOperatorInt.h',
             'compiler/translator/ScalarizeVecAndMatConstructorArgs.cpp',
             'compiler/translator/ScalarizeVecAndMatConstructorArgs.h',
             'compiler/translator/SearchSymbol.cpp',

src/compiler/translator/RewriteUnaryMinusOperatorInt.cpp

+//
+// Copyright (c) 2016 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// Implementation of evaluating unary integer variable bug workaround.
+// See header for more info.
+
+#include "compiler/translator/RewriteUnaryMinusOperatorInt.h"
+
+#include "compiler/translator/IntermNode.h"
+
+namespace sh
+{
+
+namespace
+{
+
+class Traverser : public TIntermTraverser
+{
+  public:
+    static void Apply(TIntermNode *root);
+
+  private:
+    Traverser();
+    bool visitUnary(Visit visit, TIntermUnary *node) override;
+    void nextIteration();
+
+    bool mFound = false;
+};
+
+// static
+void Traverser::Apply(TIntermNode *root)
+{
+    Traverser traverser;
+    do
+    {
+        traverser.nextIteration();
+        root->traverse(&traverser);
+        if (traverser.mFound)
+        {
+            traverser.updateTree();
+        }
+    } while (traverser.mFound);
+}
+
+Traverser::Traverser() : TIntermTraverser(true, false, false)
+{
+}
+
+void Traverser::nextIteration()
+{
+    mFound = false;
+}
+
+bool Traverser::visitUnary(Visit visit, TIntermUnary *node)
+{
+    if (mFound)
+    {
+        return false;
+    }
+
+    // Decide if the current unary operator is unary minus.
+    if (node->getOp() != EOpNegative)
+    {
+        return true;
+    }
+
+    // Decide if the current operand is an integer variable.
+    TIntermTyped *opr = node->getOperand();
+    if (!opr->getType().isScalarInt())
+    {
+        return true;
+    }
+
+    // Potential problem case detected, apply workaround: -(int) -> ~(int) + 1.
+    // ~(int)
+    TIntermUnary *bitwiseNot = new TIntermUnary(EOpBitwiseNot, opr);
+    bitwiseNot->setLine(opr->getLine());
+
+    // Constant 1 (or 1u)
+    TConstantUnion *one = new TConstantUnion();
+    if (opr->getType().getBasicType() == EbtInt)
+    {
+        one->setIConst(1);
+    }
+    else
+    {
+        one->setUConst(1u);
+    }
+    TIntermConstantUnion *oneNode = new TIntermConstantUnion(one, opr->getType());
+    oneNode->getTypePointer()->setQualifier(EvqConst);
+    oneNode->setLine(opr->getLine());
+
+    // ~(int) + 1
+    TIntermBinary *add = new TIntermBinary(EOpAdd, bitwiseNot, oneNode);
+    add->setLine(opr->getLine());
+
+    queueReplacement(node, add, OriginalNode::IS_DROPPED);
+
+    mFound = true;
+    return false;
+}
+
+}  // anonymous namespace
+
+void RewriteUnaryMinusOperatorInt(TIntermNode *root)
+{
+    Traverser::Apply(root);
+}
+
+}  // namespace sh
\ No newline at end of file

src/compiler/translator/RewriteUnaryMinusOperatorInt.h

+// Copyright (c) 2016 The ANGLE Project Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+//
+// This mutating tree traversal works around a bug on evaluating unary
+// integer variable on Intel D3D driver. It works by rewriting -(int) to
+// ~(int) + 1 when evaluating unary integer variables.
+
+#ifndef COMPILER_TRANSLATOR_REWRITEUNARYMINUSOPERATORINT_H_
+#define COMPILER_TRANSLATOR_REWRITEUNARYMINUSOPERATORINT_H_
+
+class TIntermNode;
+namespace sh
+{
+
+void RewriteUnaryMinusOperatorInt(TIntermNode *root);
+
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_REWRITEUNARYMINUSOPERATORINT_H_
\ No newline at end of file

src/compiler/translator/TranslatorHLSL.cpp

@@ -16,6 +16,7 @@
 #include "compiler/translator/RemoveDynamicIndexing.h"
 #include "compiler/translator/RewriteElseBlocks.h"
 #include "compiler/translator/RewriteTexelFetchOffset.h"
+#include "compiler/translator/RewriteUnaryMinusOperatorInt.h"
 #include "compiler/translator/SeparateArrayInitialization.h"
 #include "compiler/translator/SeparateDeclarations.h"
 #include "compiler/translator/SeparateExpressionsReturningArrays.h"
@@ -105,6 +106,12 @@ void TranslatorHLSL::translate(TIntermNode *root, ShCompileOptions compileOption
                                     getShaderVersion());
     }
 
+    if (((compileOptions & SH_REWRITE_INTEGER_UNARY_MINUS_OPERATOR) != 0) &&
+        getShaderType() == GL_VERTEX_SHADER)
+    {
+        sh::RewriteUnaryMinusOperatorInt(root);
+    }
+
     sh::OutputHLSL outputHLSL(getShaderType(), getShaderVersion(), getExtensionBehavior(),
         getSourcePath(), getOutputType(), numRenderTargets, getUniforms(), compileOptions);
 

src/libANGLE/renderer/d3d/ShaderD3D.cpp

@@ -59,6 +59,10 @@ ShaderD3D::ShaderD3D(const gl::ShaderState &data, const WorkaroundsD3D &workarou
     {
         mAdditionalOptions |= SH_REWRITE_TEXELFETCHOFFSET_TO_TEXELFETCH;
     }
+    if (workarounds.rewriteUnaryMinusOperator)
+    {
+        mAdditionalOptions |= SH_REWRITE_INTEGER_UNARY_MINUS_OPERATOR;
+    }
 }
 
 ShaderD3D::~ShaderD3D()

src/libANGLE/renderer/d3d/WorkaroundsD3D.h

@@ -72,7 +72,7 @@ struct WorkaroundsD3D
     // is negative, even if the sum of Offset and Location is in range. This may cause errors when
     // translating GLSL's function texelFetchOffset into texture.Load, as it is valid for
     // texelFetchOffset to use negative texture coordinates as its parameter P when the sum of P
-    // and Offset is in range. To work around this, we translatie texelFetchOffset into texelFetch
+    // and Offset is in range. To work around this, we translate texelFetchOffset into texelFetch
     // by adding Offset directly to Location before reading the texture.
     bool preAddTexelFetchOffsets = false;
 
@@ -85,6 +85,10 @@ struct WorkaroundsD3D
     // This workaroud will disable B5G6R5 support when it's Intel driver. By default, it will use
     // R8G8B8A8 format.
     bool disableB5G6R5Support = false;
+
+    // On some Intel drivers, evaluating unary minus operator on integer may get wrong answer in
+    // vertex shaders. To work around this bug, we translate -(int) into ~(int)+1.
+    bool rewriteUnaryMinusOperator = false;
 };
 
 }  // namespace rx

src/libANGLE/renderer/d3d/d3d11/renderer11_utils.cpp

@@ -1542,6 +1542,7 @@ WorkaroundsD3D GenerateWorkarounds(const Renderer11DeviceCaps &deviceCaps,
 
     workarounds.preAddTexelFetchOffsets = (adapterDesc.VendorId == VENDOR_ID_INTEL);
     workarounds.disableB5G6R5Support    = (adapterDesc.VendorId == VENDOR_ID_INTEL);
+    workarounds.rewriteUnaryMinusOperator = (adapterDesc.VendorId == VENDOR_ID_INTEL);
 
     // TODO(jmadill): Disable when we have a fixed driver version.
     workarounds.emulateTinyStencilTextures = (adapterDesc.VendorId == VENDOR_ID_AMD);

src/tests/gl_tests/GLSLTest.cpp

@@ -2241,6 +2241,104 @@ TEST_P(GLSLTest, NestedPowStatements)
     EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
 }
 
+// Convers a bug with the unary minus operator on signed integer workaround.
+TEST_P(GLSLTest_ES3, UnaryMinusOperatorSignedInt)
+{
+    const std::string &vert =
+        "#version 300 es\n"
+        "in highp vec4 position;\n"
+        "out mediump vec4 v_color;\n"
+        "uniform int ui_one;\n"
+        "uniform int ui_two;\n"
+        "uniform int ui_three;\n"
+        "void main() {\n"
+        "    int s[3];\n"
+        "    s[0] = ui_one;\n"
+        "    s[1] = -(-(-ui_two + 1) + 1);\n"  // s[1] = -ui_two
+        "    s[2] = ui_three;\n"
+        "    int result = 0;\n"
+        "    for (int i = 0; i < ui_three; i++) {\n"
+        "        result += s[i];\n"
+        "    }\n"
+        "    v_color = (result == 2) ? vec4(0, 1, 0, 1) : vec4(1, 0, 0, 1);\n"
+        "    gl_Position = position;\n"
+        "}\n";
+    const std::string &frag =
+        "#version 300 es\n"
+        "in mediump vec4 v_color;\n"
+        "layout(location=0) out mediump vec4 o_color;\n"
+        "void main() {\n"
+        "    o_color = v_color;\n"
+        "}\n";
+
+    ANGLE_GL_PROGRAM(prog, vert, frag);
+
+    gl::Context *context   = reinterpret_cast<gl::Context *>(getEGLWindow()->getContext());
+    gl::Program *glProgram = context->getProgram(prog.get());
+    GLint oneIndex         = glProgram->getUniformLocation("ui_one");
+    ASSERT_NE(-1, oneIndex);
+    GLint twoIndex = glProgram->getUniformLocation("ui_two");
+    ASSERT_NE(-1, twoIndex);
+    GLint threeIndex = glProgram->getUniformLocation("ui_three");
+    ASSERT_NE(-1, threeIndex);
+    glUseProgram(prog.get());
+    glUniform1i(oneIndex, 1);
+    glUniform1i(twoIndex, 2);
+    glUniform1i(threeIndex, 3);
+
+    drawQuad(prog.get(), "position", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
+// Convers a bug with the unary minus operator on unsigned integer workaround.
+TEST_P(GLSLTest_ES3, UnaryMinusOperatorUnsignedInt)
+{
+    const std::string &vert =
+        "#version 300 es\n"
+        "in highp vec4 position;\n"
+        "out mediump vec4 v_color;\n"
+        "uniform uint ui_one;\n"
+        "uniform uint ui_two;\n"
+        "uniform uint ui_three;\n"
+        "void main() {\n"
+        "    uint s[3];\n"
+        "    s[0] = ui_one;\n"
+        "    s[1] = -(-(-ui_two + 1u) + 1u);\n"  // s[1] = -ui_two
+        "    s[2] = ui_three;\n"
+        "    uint result = 0u;\n"
+        "    for (uint i = 0u; i < ui_three; i++) {\n"
+        "        result += s[i];\n"
+        "    }\n"
+        "    v_color = (result == 2u) ? vec4(0, 1, 0, 1) : vec4(1, 0, 0, 1);\n"
+        "    gl_Position = position;\n"
+        "}\n";
+    const std::string &frag =
+        "#version 300 es\n"
+        "in mediump vec4 v_color;\n"
+        "layout(location=0) out mediump vec4 o_color;\n"
+        "void main() {\n"
+        "    o_color = v_color;\n"
+        "}\n";
+
+    ANGLE_GL_PROGRAM(prog, vert, frag);
+
+    gl::Context *context   = reinterpret_cast<gl::Context *>(getEGLWindow()->getContext());
+    gl::Program *glProgram = context->getProgram(prog.get());
+    GLint oneIndex         = glProgram->getUniformLocation("ui_one");
+    ASSERT_NE(-1, oneIndex);
+    GLint twoIndex = glProgram->getUniformLocation("ui_two");
+    ASSERT_NE(-1, twoIndex);
+    GLint threeIndex = glProgram->getUniformLocation("ui_three");
+    ASSERT_NE(-1, threeIndex);
+    glUseProgram(prog.get());
+    glUniform1ui(oneIndex, 1u);
+    glUniform1ui(twoIndex, 2u);
+    glUniform1ui(threeIndex, 3u);
+
+    drawQuad(prog.get(), "position", 0.5f);
+    EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+}
+
 // Test a nested sequence operator with a ternary operator inside. The ternary operator is
 // intended to be such that it gets converted to an if statement on the HLSL backend.
 TEST_P(GLSLTest, NestedSequenceOperatorWithTernaryInside)