Disable shader's pre-rotation code on Metal & non-Android.

Pre-rotation code were added to transform gl_Position, gl_FragCoords,
gl_PointCoords, dFdX, dFdY in shader. However, it is only useful for
android's surface pre-rotation and completely un-needed in Metal
back-end.

This CL disables these pre-rotation code if the platform is not
android.

Bug: angleproject:4678
Change-Id: I89c42fcf24b49896f4ed9c2f9465da521beaf25f
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/2295000
Commit-Queue: Shahbaz Youssefi <[email protected]>
Reviewed-by: Jamie Madill <[email protected]>
Reviewed-by: Shahbaz Youssefi <[email protected]>
Reviewed-by: Ian Elliott <[email protected]>

Author: kakashidinho

include/GLSLANG/ShaderLang.h

@@ -26,7 +26,7 @@
 
 // Version number for shader translation API.
 // It is incremented every time the API changes.
-#define ANGLE_SH_VERSION 231
+#define ANGLE_SH_VERSION 232
 
 enum ShShaderSpec
 {
@@ -339,6 +339,9 @@ const ShCompileOptions SH_IGNORE_PRECISION_QUALIFIERS = UINT64_C(1) << 54;
 // Allow compiler to do early fragment tests as an optimization.
 const ShCompileOptions SH_EARLY_FRAGMENT_TESTS_OPTIMIZATION = UINT64_C(1) << 55;
 
+// Allow compiler to insert Android pre-rotation code.
+const ShCompileOptions SH_ADD_PRE_ROTATION = UINT64_C(1) << 56;
+
 // Defines alternate strategies for implementing array index clamping.
 enum ShArrayIndexClampingStrategy
 {

src/compiler/translator/TranslatorVulkan.cpp

@@ -242,8 +242,17 @@ ANGLE_NO_DISCARD bool RotateAndFlipBuiltinVariable(TCompiler *compiler,
     }
 
     // Create the expression "(builtin.xy * fragRotation)"
-    TIntermBinary *rotatedXY =
-        new TIntermBinary(EOpMatrixTimesVector, fragRotation->deepCopy(), builtinXY->deepCopy());
+    TIntermTyped *rotatedXY;
+    if (fragRotation)
+    {
+        rotatedXY = new TIntermBinary(EOpMatrixTimesVector, fragRotation->deepCopy(),
+                                      builtinXY->deepCopy());
+    }
+    else
+    {
+        // No rotation applied, use original variable.
+        rotatedXY = builtinXY->deepCopy();
+    }
 
     // Create the expression "(builtin.xy - pivot) * flipXY + pivot
     TIntermBinary *removePivot = new TIntermBinary(EOpSub, rotatedXY, pivot);
@@ -571,14 +580,17 @@ ANGLE_NO_DISCARD bool AddBresenhamEmulationVS(TCompiler *compiler,
 }
 
 ANGLE_NO_DISCARD bool InsertFragCoordCorrection(TCompiler *compiler,
+                                                ShCompileOptions compileOptions,
                                                 TIntermBlock *root,
                                                 TIntermSequence *insertSequence,
                                                 TSymbolTable *symbolTable,
                                                 const TVariable *driverUniforms)
 {
     TIntermBinary *flipXY       = CreateDriverUniformRef(driverUniforms, kFlipXY);
     TIntermBinary *pivot        = CreateDriverUniformRef(driverUniforms, kHalfRenderArea);
-    TIntermBinary *fragRotation = CreateDriverUniformRef(driverUniforms, kFragRotation);
+    TIntermBinary *fragRotation = (compileOptions & SH_ADD_PRE_ROTATION)
+                                      ? CreateDriverUniformRef(driverUniforms, kFragRotation)
+                                      : nullptr;
     return RotateAndFlipBuiltinVariable(compiler, root, insertSequence, flipXY, symbolTable,
                                         BuiltInVariable::gl_FragCoord(), kFlippedFragCoordName,
                                         pivot, fragRotation);
@@ -618,6 +630,7 @@ ANGLE_NO_DISCARD bool InsertFragCoordCorrection(TCompiler *compiler,
 // Note this emulation can not provide fully correct rasterization. See the docs more more info.
 
 ANGLE_NO_DISCARD bool AddBresenhamEmulationFS(TCompiler *compiler,
+                                              ShCompileOptions compileOptions,
                                               TInfoSinkBase &sink,
                                               TIntermBlock *root,
                                               TSymbolTable *symbolTable,
@@ -718,8 +731,8 @@ ANGLE_NO_DISCARD bool AddBresenhamEmulationFS(TCompiler *compiler,
     // If the shader does not use frag coord, we should insert it inside the emulation if.
     if (!usesFragCoord)
     {
-        if (!InsertFragCoordCorrection(compiler, root, emulationSequence, symbolTable,
-                                       driverUniforms))
+        if (!InsertFragCoordCorrection(compiler, compileOptions, root, emulationSequence,
+                                       symbolTable, driverUniforms))
         {
             return false;
         }
@@ -933,15 +946,16 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
 
         if (compileOptions & SH_ADD_BRESENHAM_LINE_RASTER_EMULATION)
         {
-            if (!AddBresenhamEmulationFS(this, sink, root, &getSymbolTable(), driverUniforms,
-                                         usesFragCoord))
+            if (!AddBresenhamEmulationFS(this, compileOptions, sink, root, &getSymbolTable(),
+                                         driverUniforms, usesFragCoord))
             {
                 return false;
             }
         }
 
         bool hasGLFragColor = false;
         bool hasGLFragData  = false;
+        bool usePreRotation = compileOptions & SH_ADD_PRE_ROTATION;
 
         for (const ShaderVariable &outputVar : mOutputVariables)
         {
@@ -972,7 +986,8 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
         {
             TIntermBinary *flipXY       = CreateDriverUniformRef(driverUniforms, kNegFlipXY);
             TIntermConstantUnion *pivot = CreateFloatNode(0.5f);
-            TIntermBinary *fragRotation = CreateDriverUniformRef(driverUniforms, kFragRotation);
+            TIntermBinary *fragRotation =
+                usePreRotation ? CreateDriverUniformRef(driverUniforms, kFragRotation) : nullptr;
             if (!RotateAndFlipBuiltinVariable(this, root, GetMainSequence(root), flipXY,
                                               &getSymbolTable(), BuiltInVariable::gl_PointCoord(),
                                               kFlippedPointCoordName, pivot, fragRotation))
@@ -983,16 +998,17 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
 
         if (usesFragCoord)
         {
-            if (!InsertFragCoordCorrection(this, root, GetMainSequence(root), &getSymbolTable(),
-                                           driverUniforms))
+            if (!InsertFragCoordCorrection(this, compileOptions, root, GetMainSequence(root),
+                                           &getSymbolTable(), driverUniforms))
             {
                 return false;
             }
         }
 
         {
-            TIntermBinary *flipXY       = CreateDriverUniformRef(driverUniforms, kFlipXY);
-            TIntermBinary *fragRotation = CreateDriverUniformRef(driverUniforms, kFragRotation);
+            TIntermBinary *flipXY = CreateDriverUniformRef(driverUniforms, kFlipXY);
+            TIntermBinary *fragRotation =
+                usePreRotation ? CreateDriverUniformRef(driverUniforms, kFragRotation) : nullptr;
             if (!RewriteDfdy(this, root, getSymbolTable(), getShaderVersion(), flipXY,
                              fragRotation))
             {
@@ -1047,7 +1063,8 @@ bool TranslatorVulkan::translateImpl(TIntermBlock *root,
         {
             return false;
         }
-        if (!AppendPreRotation(this, root, &getSymbolTable(), driverUniforms))
+        if ((compileOptions & SH_ADD_PRE_ROTATION) != 0 &&
+            !AppendPreRotation(this, root, &getSymbolTable(), driverUniforms))
         {
             return false;
         }

src/compiler/translator/tree_ops/RewriteDfdy.cpp

@@ -33,6 +33,9 @@ class Traverser : public TIntermTraverser
     Traverser(TIntermBinary *flipXY, TIntermTyped *fragRotation, TSymbolTable *symbolTable);
     bool visitUnary(Visit visit, TIntermUnary *node) override;
 
+    bool visitUnaryWithRotation(Visit visit, TIntermUnary *node);
+    bool visitUnaryWithoutRotation(Visit visit, TIntermUnary *node);
+
     TIntermBinary *mFlipXY      = nullptr;
     TIntermTyped *mFragRotation = nullptr;
 };
@@ -57,6 +60,15 @@ bool Traverser::Apply(TCompiler *compiler,
 }
 
 bool Traverser::visitUnary(Visit visit, TIntermUnary *node)
+{
+    if (mFragRotation)
+    {
+        return visitUnaryWithRotation(visit, node);
+    }
+    return visitUnaryWithoutRotation(visit, node);
+}
+
+bool Traverser::visitUnaryWithRotation(Visit visit, TIntermUnary *node)
 {
     // Decide if the node represents a call to dFdx() or dFdy()
     if ((node->getOp() != EOpDFdx) && (node->getOp() != EOpDFdy))
@@ -140,6 +152,32 @@ bool Traverser::visitUnary(Visit visit, TIntermUnary *node)
     return true;
 }
 
+bool Traverser::visitUnaryWithoutRotation(Visit visit, TIntermUnary *node)
+{
+    // Decide if the node represents a call to dFdy()
+    if (node->getOp() != EOpDFdy)
+    {
+        return true;
+    }
+
+    // Copy the dFdy node so we can replace it with the corrected value
+    TIntermUnary *newDfdy = node->deepCopy()->getAsUnaryNode();
+
+    size_t objectSize    = node->getType().getObjectSize();
+    TOperator multiplyOp = (objectSize == 1) ? EOpMul : EOpVectorTimesScalar;
+
+    TIntermBinary *flipY =
+        new TIntermBinary(EOpIndexDirect, mFlipXY->deepCopy(), CreateIndexNode(1));
+    // Correct dFdy()'s value:
+    // (dFdy() * mFlipXY.y)
+    TIntermBinary *correctedDfdy = new TIntermBinary(multiplyOp, newDfdy, flipY);
+
+    // Replace the old dFdy node with the new node that contains the corrected value
+    queueReplacement(correctedDfdy, OriginalNode::IS_DROPPED);
+
+    return true;
+}
+
 }  // anonymous namespace
 
 bool RewriteDfdy(TCompiler *compiler,

src/compiler/translator/tree_ops/RewriteDfdy.h

@@ -24,6 +24,7 @@ class TIntermBinary;
 class TIntermTyped;
 class TSymbolTable;
 
+// If fragRotation = nullptr, no rotation will be applied.
 ANGLE_NO_DISCARD bool RewriteDfdy(TCompiler *compiler,
                                   TIntermNode *root,
                                   const TSymbolTable &symbolTable,

src/libANGLE/renderer/metal/ContextMtl.h

@@ -442,11 +442,13 @@ class ContextMtl : public ContextImpl, public mtl::Context
 
         // Used to pre-rotate gl_Position for Vulkan swapchain images on Android (a mat2, which is
         // padded to the size of two vec4's).
-        float preRotation[8];
+        // Unused in Metal.
+        float preRotation[8] = {};
 
         // Used to pre-rotate gl_FragCoord for Vulkan swapchain images on Android (a mat2, which is
         // padded to the size of two vec4's).
-        float fragRotation[8];
+        // Unused in Metal.
+        float fragRotation[8] = {};
 
         uint32_t coverageMask;
 

src/libANGLE/renderer/metal/ContextMtl.mm

@@ -1702,25 +1702,7 @@ bool NeedToInvertDepthRange(float near, float far)
     mDriverUniforms.depthRange[2] = depthRangeDiff;
     mDriverUniforms.depthRange[3] = NeedToInvertDepthRange(depthRangeNear, depthRangeFar) ? -1 : 1;
 
-    // Fill in a mat2 identity matrix, plus padding
-    mDriverUniforms.preRotation[0] = 1.0f;
-    mDriverUniforms.preRotation[1] = 0.0f;
-    mDriverUniforms.preRotation[2] = 0.0f;
-    mDriverUniforms.preRotation[3] = 0.0f;
-    mDriverUniforms.preRotation[4] = 0.0f;
-    mDriverUniforms.preRotation[5] = 1.0f;
-    mDriverUniforms.preRotation[6] = 0.0f;
-    mDriverUniforms.preRotation[7] = 0.0f;
-
-    // Fill in a mat2 identity matrix, plus padding
-    mDriverUniforms.fragRotation[0] = 1.0f;
-    mDriverUniforms.fragRotation[1] = 0.0f;
-    mDriverUniforms.fragRotation[2] = 0.0f;
-    mDriverUniforms.fragRotation[3] = 0.0f;
-    mDriverUniforms.fragRotation[4] = 0.0f;
-    mDriverUniforms.fragRotation[5] = 1.0f;
-    mDriverUniforms.fragRotation[6] = 0.0f;
-    mDriverUniforms.fragRotation[7] = 0.0f;
+    // NOTE(hqle): preRotation & fragRotation are unused.
 
     // Sample coverage mask
     uint32_t sampleBitCount = mDrawFramebuffer->getSamples();

src/libANGLE/renderer/vulkan/ShaderVk.cpp

@@ -64,6 +64,12 @@ std::shared_ptr<WaitableCompileEvent> ShaderVk::compile(const gl::Context *conte
     // context state does not allow it
     compileOptions |= SH_EARLY_FRAGMENT_TESTS_OPTIMIZATION;
 
+    if (contextVk->getFeatures().enablePreRotateSurfaces.enabled)
+    {
+        // Let compiler inserts pre-rotation code.
+        compileOptions |= SH_ADD_PRE_ROTATION;
+    }
+
     return compileImpl(context, compilerInstance, mData.getSource(), compileOptions | options);
 }
 

src/tests/egl_tests/EGLPreRotationTest.cpp

@@ -23,6 +23,24 @@ using namespace angle;
 namespace
 {
 
+using EGLPreRotationSurfaceTestParams = std::tuple<angle::PlatformParameters, bool>;
+
+std::string PrintToStringParamName(
+    const ::testing::TestParamInfo<EGLPreRotationSurfaceTestParams> &info)
+{
+    std::stringstream ss;
+    ss << std::get<0>(info.param);
+    if (std::get<1>(info.param))
+    {
+        ss << "__PreRotationEnabled";
+    }
+    else
+    {
+        ss << "__PreRotationDisabled";
+    }
+    return ss.str();
+}
+
 // A class to test various Android pre-rotation cases.  In order to make it easier to debug test
 // failures, the initial window size is 256x256, and each pixel will have a unique and predictable
 // value.  The red channel will increment with the x axis, and the green channel will increment
@@ -33,7 +51,7 @@ namespace
 // Lower-right, which is ( 1.0,-1.0) & (256,   0) in GLES will be red    (0xFF, 0x00, 0x00, 0xFF)
 // Upper-left,  which is (-1.0, 1.0) & (  0, 256) in GLES will be green  (0x00, 0xFF, 0x00, 0xFF)
 // Upper-right, which is ( 1.0, 1.0) & (256, 256) in GLES will be yellow (0xFF, 0xFF, 0x00, 0xFF)
-class EGLPreRotationSurfaceTest : public ANGLETest
+class EGLPreRotationSurfaceTest : public ANGLETestWithParam<EGLPreRotationSurfaceTestParams>
 {
   protected:
     EGLPreRotationSurfaceTest()
@@ -81,10 +99,24 @@ class EGLPreRotationSurfaceTest : public ANGLETest
 
     void initializeDisplay()
     {
-        GLenum platformType = GetParam().getRenderer();
-        GLenum deviceType   = GetParam().getDeviceType();
+        const angle::PlatformParameters platform = ::testing::get<0>(GetParam());
+        GLenum platformType                      = platform.getRenderer();
+        GLenum deviceType                        = platform.getDeviceType();
+
+        std::vector<const char *> enabledFeatures;
+        std::vector<const char *> disabledFeatures;
+        if (::testing::get<1>(GetParam()))
+        {
+            enabledFeatures.push_back("enable_pre_rotation_surfaces");
+        }
+        else
+        {
+            disabledFeatures.push_back("enable_pre_rotation_surfaces");
+        }
+        enabledFeatures.push_back(nullptr);
+        disabledFeatures.push_back(nullptr);
 
-        std::vector<EGLint> displayAttributes;
+        std::vector<EGLAttrib> displayAttributes;
         displayAttributes.push_back(EGL_PLATFORM_ANGLE_TYPE_ANGLE);
         displayAttributes.push_back(platformType);
         displayAttributes.push_back(EGL_PLATFORM_ANGLE_MAX_VERSION_MAJOR_ANGLE);
@@ -93,11 +125,15 @@ class EGLPreRotationSurfaceTest : public ANGLETest
         displayAttributes.push_back(EGL_DONT_CARE);
         displayAttributes.push_back(EGL_PLATFORM_ANGLE_DEVICE_TYPE_ANGLE);
         displayAttributes.push_back(deviceType);
+        displayAttributes.push_back(EGL_FEATURE_OVERRIDES_ENABLED_ANGLE);
+        displayAttributes.push_back(reinterpret_cast<EGLAttrib>(enabledFeatures.data()));
+        displayAttributes.push_back(EGL_FEATURE_OVERRIDES_DISABLED_ANGLE);
+        displayAttributes.push_back(reinterpret_cast<EGLAttrib>(disabledFeatures.data()));
         displayAttributes.push_back(EGL_NONE);
 
-        mDisplay = eglGetPlatformDisplayEXT(EGL_PLATFORM_ANGLE_ANGLE,
-                                            reinterpret_cast<void *>(mOSWindow->getNativeDisplay()),
-                                            displayAttributes.data());
+        mDisplay = eglGetPlatformDisplay(EGL_PLATFORM_ANGLE_ANGLE,
+                                         reinterpret_cast<void *>(mOSWindow->getNativeDisplay()),
+                                         displayAttributes.data());
         ASSERT_TRUE(mDisplay != EGL_NO_DISPLAY);
 
         EGLint majorVersion, minorVersion;
@@ -109,7 +145,8 @@ class EGLPreRotationSurfaceTest : public ANGLETest
 
     void initializeContext()
     {
-        EGLint contextAttibutes[] = {EGL_CONTEXT_CLIENT_VERSION, GetParam().majorVersion, EGL_NONE};
+        EGLint contextAttibutes[] = {EGL_CONTEXT_CLIENT_VERSION,
+                                     ::testing::get<0>(GetParam()).majorVersion, EGL_NONE};
 
         mContext = eglCreateContext(mDisplay, mConfig, nullptr, contextAttibutes);
         ASSERT_EGL_SUCCESS();
@@ -1582,12 +1619,26 @@ TEST_P(EGLPreRotationBlitFramebufferTest, FboDestOutOfBoundsSourceAndDestBlitFra
 }
 }  // anonymous namespace
 
-ANGLE_INSTANTIATE_TEST(EGLPreRotationSurfaceTest,
-                       WithNoFixture(ES2_VULKAN()),
-                       WithNoFixture(ES3_VULKAN()));
-ANGLE_INSTANTIATE_TEST(EGLPreRotationLargeSurfaceTest,
-                       WithNoFixture(ES2_VULKAN()),
-                       WithNoFixture(ES3_VULKAN()));
-ANGLE_INSTANTIATE_TEST(EGLPreRotationBlitFramebufferTest,
-                       WithNoFixture(ES2_VULKAN()),
-                       WithNoFixture(ES3_VULKAN()));
+#ifdef Bool
+// X11 ridiculousness.
+#    undef Bool
+#endif
+
+ANGLE_INSTANTIATE_TEST_COMBINE_1(EGLPreRotationSurfaceTest,
+                                 PrintToStringParamName,
+                                 testing::Bool(),
+                                 WithNoFixture(ES2_VULKAN()),
+                                 WithNoFixture(ES3_VULKAN()),
+                                 WithNoFixture(ES3_VULKAN_SWIFTSHADER()));
+ANGLE_INSTANTIATE_TEST_COMBINE_1(EGLPreRotationLargeSurfaceTest,
+                                 PrintToStringParamName,
+                                 testing::Bool(),
+                                 WithNoFixture(ES2_VULKAN()),
+                                 WithNoFixture(ES3_VULKAN()),
+                                 WithNoFixture(ES3_VULKAN_SWIFTSHADER()));
+ANGLE_INSTANTIATE_TEST_COMBINE_1(EGLPreRotationBlitFramebufferTest,
+                                 PrintToStringParamName,
+                                 testing::Bool(),
+                                 WithNoFixture(ES2_VULKAN()),
+                                 WithNoFixture(ES3_VULKAN()),
+                                 WithNoFixture(ES3_VULKAN_SWIFTSHADER()));