Clamp point size to maximum point size from the API on NVIDIA

include/GLSLANG/ShaderLang.h

@@ -25,7 +25,7 @@
 
 // Version number for shader translation API.
 // It is incremented every time the API changes.
-#define ANGLE_SH_VERSION 177
+#define ANGLE_SH_VERSION 178
 
 enum ShShaderSpec
 {
@@ -240,6 +240,10 @@ const ShCompileOptions SH_INITIALIZE_BUILTINS_FOR_INSTANCED_MULTIVIEW = UINT64_C
 // temporary variable ViewID_OVR declared and initialized.
 const ShCompileOptions SH_SELECT_VIEW_IN_NV_GLSL_VERTEX_SHADER = UINT64_C(1) << 34;
 
+// If the flag is enabled, gl_PointSize is clamped to the maximum point size specified in
+// ShBuiltInResources in vertex shaders.
+const ShCompileOptions SH_CLAMP_POINT_SIZE = UINT64_C(1) << 35;
+
 // Defines alternate strategies for implementing array index clamping.
 enum ShArrayIndexClampingStrategy
 {
@@ -404,6 +408,9 @@ struct ShBuiltInResources
 
     // maximum number of shader storage buffer bindings
     int MaxShaderStorageBufferBindings;
+
+    // maximum point size (higher limit from ALIASED_POINT_SIZE_RANGE)
+    float MaxPointSize;
 };
 
 //

src/compiler.gypi

@@ -33,6 +33,8 @@
             'compiler/translator/Cache.h',
             'compiler/translator/CallDAG.cpp',
             'compiler/translator/CallDAG.h',
+            'compiler/translator/ClampPointSize.cpp',
+            'compiler/translator/ClampPointSize.h',
             'compiler/translator/CodeGen.cpp',
             'compiler/translator/Common.h',
             'compiler/translator/Compiler.cpp',

src/compiler/translator/ClampPointSize.cpp

+//
+// Copyright (c) 2017 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.
+//
+// ClampPointSize.cpp: Limit the value that is written to gl_PointSize.
+//
+
+#include "compiler/translator/ClampPointSize.h"
+
+#include "compiler/translator/FindSymbolNode.h"
+#include "compiler/translator/IntermNode_util.h"
+#include "compiler/translator/RunAtTheEndOfShader.h"
+#include "compiler/translator/SymbolTable.h"
+
+namespace sh
+{
+
+void ClampPointSize(TIntermBlock *root, float maxPointSize, TSymbolTable *symbolTable)
+{
+    // Only clamp gl_PointSize if it's used in the shader.
+    if (!FindSymbolNode(root, TString("gl_PointSize"), EbtFloat))
+    {
+        return;
+    }
+
+    TIntermSymbol *pointSizeNode = ReferenceBuiltInVariable("gl_PointSize", *symbolTable, 100);
+
+    TConstantUnion *maxPointSizeConstant = new TConstantUnion();
+    maxPointSizeConstant->setFConst(maxPointSize);
+    TIntermConstantUnion *maxPointSizeNode =
+        new TIntermConstantUnion(maxPointSizeConstant, TType(EbtFloat, EbpHigh, EvqConst));
+
+    // min(gl_PointSize, maxPointSize)
+    TIntermSequence *minArguments = new TIntermSequence();
+    minArguments->push_back(pointSizeNode->deepCopy());
+    minArguments->push_back(maxPointSizeNode);
+    TIntermTyped *clampedPointSize =
+        CreateBuiltInFunctionCallNode("min", minArguments, *symbolTable, 100);
+
+    // gl_PointSize = min(gl_PointSize, maxPointSize)
+    TIntermBinary *assignPointSize = new TIntermBinary(EOpAssign, pointSizeNode, clampedPointSize);
+
+    RunAtTheEndOfShader(root, assignPointSize, symbolTable);
+}
+
+}  // namespace sh

src/compiler/translator/ClampPointSize.h

+//
+// Copyright (c) 2017 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.
+//
+// ClampPointSize.h: Limit the value that is written to gl_PointSize.
+//
+
+#ifndef COMPILER_TRANSLATOR_CLAMPPOINTSIZE_H_
+#define COMPILER_TRANSLATOR_CLAMPPOINTSIZE_H_
+
+namespace sh
+{
+
+class TIntermBlock;
+class TSymbolTable;
+
+void ClampPointSize(TIntermBlock *root, float maxPointSize, TSymbolTable *symbolTable);
+
+}  // namespace sh
+
+#endif  // COMPILER_TRANSLATOR_CLAMPPOINTSIZE_H_

src/compiler/translator/Compiler.cpp

@@ -13,6 +13,7 @@
 #include "compiler/translator/AddAndTrueToLoopCondition.h"
 #include "compiler/translator/Cache.h"
 #include "compiler/translator/CallDAG.h"
+#include "compiler/translator/ClampPointSize.h"
 #include "compiler/translator/DeclareAndInitBuiltinsForInstancedMultiview.h"
 #include "compiler/translator/DeferGlobalInitializers.h"
 #include "compiler/translator/EmulateGLFragColorBroadcast.h"
@@ -547,6 +548,12 @@ TIntermBlock *TCompiler::compileTreeImpl(const char *const shaderStrings[],
             SeparateDeclarations(root);
             InitializeUninitializedLocals(root, getShaderVersion());
         }
+
+        if (success && getShaderType() == GL_VERTEX_SHADER &&
+            (compileOptions & SH_CLAMP_POINT_SIZE))
+        {
+            ClampPointSize(root, compileResources.MaxPointSize, &getSymbolTable());
+        }
     }
 
     if (success)

src/compiler/translator/IntermNode_util.cpp

@@ -24,6 +24,21 @@ TName GetInternalFunctionName(const char *name)
     return nameObj;
 }
 
+const TFunction *LookUpBuiltInFunction(const TString &name,
+                                       const TIntermSequence *arguments,
+                                       const TSymbolTable &symbolTable,
+                                       int shaderVersion)
+{
+    TString mangledName = TFunction::GetMangledNameFromCall(name, *arguments);
+    TSymbol *symbol     = symbolTable.findBuiltIn(mangledName, shaderVersion);
+    if (symbol)
+    {
+        ASSERT(symbol->isFunction());
+        return static_cast<const TFunction *>(symbol);
+    }
+    return nullptr;
+}
+
 }  // anonymous namespace
 
 TIntermFunctionPrototype *CreateInternalFunctionPrototypeNode(const TType &returnType,
@@ -183,4 +198,23 @@ TIntermSymbol *ReferenceBuiltInVariable(const TString &name,
     return new TIntermSymbol(var->getUniqueId(), name, var->getType());
 }
 
+TIntermTyped *CreateBuiltInFunctionCallNode(const TString &name,
+                                            TIntermSequence *arguments,
+                                            const TSymbolTable &symbolTable,
+                                            int shaderVersion)
+{
+    const TFunction *fn = LookUpBuiltInFunction(name, arguments, symbolTable, shaderVersion);
+    ASSERT(fn);
+    TOperator op = fn->getBuiltInOp();
+    if (op != EOpNull)
+    {
+        if (arguments->size() == 1)
+        {
+            return new TIntermUnary(op, arguments->at(0)->getAsTyped());
+        }
+        return TIntermAggregate::Create(fn->getReturnType(), op, arguments);
+    }
+    return TIntermAggregate::CreateBuiltInFunctionCall(*fn, arguments);
+}
+
 }  // namespace sh

src/compiler/translator/IntermNode_util.h

@@ -40,6 +40,11 @@ TIntermSymbol *ReferenceBuiltInVariable(const TString &name,
                                         const TSymbolTable &symbolTable,
                                         int shaderVersion);
 
+TIntermTyped *CreateBuiltInFunctionCallNode(const TString &name,
+                                            TIntermSequence *arguments,
+                                            const TSymbolTable &symbolTable,
+                                            int shaderVersion);
+
 }  // namespace sh
 
 #endif  // COMPILER_TRANSLATOR_INTERMNODEUTIL_H_
\ No newline at end of file

src/compiler/translator/RewriteTexelFetchOffset.cpp

@@ -130,13 +130,8 @@ bool Traverser::visitAggregate(Visit visit, TIntermAggregate *node)
 
     ASSERT(texelFetchArguments->size() == 3u);
 
-    // Get the symbol of the texel fetch function to use.
-    TString mangledName = TFunction::GetMangledNameFromCall("texelFetch", *texelFetchArguments);
-    TSymbol *texelFetchSymbol = symbolTable->findBuiltIn(mangledName, shaderVersion);
-    ASSERT(texelFetchSymbol && texelFetchSymbol->isFunction());
-
-    TIntermAggregate *texelFetchNode = TIntermAggregate::CreateBuiltInFunctionCall(
-        *static_cast<const TFunction *>(texelFetchSymbol), texelFetchArguments);
+    TIntermTyped *texelFetchNode = CreateBuiltInFunctionCallNode("texelFetch", texelFetchArguments,
+                                                                 *symbolTable, shaderVersion);
     texelFetchNode->setLine(node->getLine());
 
     // Replace the old node by this new node.

src/libANGLE/Compiler.cpp

@@ -118,6 +118,9 @@ Compiler::Compiler(rx::GLImplFactory *implFactory, const ContextState &state)
 
     mResources.MaxUniformBufferBindings = caps.maxUniformBufferBindings;
 
+    // Needed by point size clamping workaround
+    mResources.MaxPointSize = caps.maxAliasedPointSize;
+
     if (state.getClientMajorVersion() == 2 && !extensions.drawBuffers)
     {
         mResources.MaxDrawBuffers = 1;

src/libANGLE/renderer/gl/ShaderGL.cpp

@@ -112,6 +112,11 @@ ShCompileOptions ShaderGL::prepareSourceAndReturnOptions(std::stringstream *sour
         options |= SH_INITIALIZE_UNINITIALIZED_LOCALS;
     }
 
+    if (mWorkarounds.clampPointSize)
+    {
+        options |= SH_CLAMP_POINT_SIZE;
+    }
+
     if (mMultiviewImplementationType == MultiviewImplementationTypeGL::NV_VIEWPORT_ARRAY2)
     {
         options |= SH_INITIALIZE_BUILTINS_FOR_INSTANCED_MULTIVIEW;

src/libANGLE/renderer/gl/WorkaroundsGL.h

@@ -129,6 +129,10 @@ struct WorkaroundsGL
     // Initializing uninitialized locals caused odd behavior on Mac in a few WebGL 2 tests.
     // Tracking bug: http://anglebug/2041
     bool dontInitializeUninitializedLocals = false;
+
+    // On some NVIDIA drivers the point size range reported from the API is inconsistent with the
+    // actual behavior. Clamp the point size to the value from the API to fix this.
+    bool clampPointSize = false;
 };
 }  // namespace rx
 

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -1053,9 +1053,13 @@ void GenerateWorkarounds(const FunctionsGL *functions, WorkaroundsGL *workaround
 
     workarounds->reapplyUBOBindingsAfterUsingBinaryProgram = IsAMD(vendor);
 
+    workarounds->clampPointSize = IsNvidia(vendor);
+
 #if defined(ANGLE_PLATFORM_ANDROID)
     // TODO(jmadill): Narrow workaround range for specific devices.
     workarounds->reapplyUBOBindingsAfterUsingBinaryProgram = true;
+
+    workarounds->clampPointSize = true;
 #endif
 }
 

src/tests/gl_tests/PointSpritesTest.cpp

@@ -7,6 +7,7 @@
 // conformance test suite.
 
 #include "test_utils/ANGLETest.h"
+#include "test_utils/gl_raii.h"
 
 #include <cmath>
 
@@ -61,9 +62,7 @@ TEST_P(PointSpritesTest, PointCoordAndPointSizeCompliance)
             gl_Position  = vPosition;
         });
 
-    GLuint program = CompileProgram(vs, fs);
-    ASSERT_NE(program, 0u);
-    ASSERT_GL_NO_ERROR();
+    ANGLE_GL_PROGRAM(program, vs, fs);
 
     glUseProgram(program);
 
@@ -88,12 +87,9 @@ TEST_P(PointSpritesTest, PointCoordAndPointSizeCompliance)
     ASSERT_GL_NO_ERROR();
 
     GLfloat pixelOffset = ((int)maxPointSize % 2) ? (1.0f / (GLfloat)windowWidth) : 0;
-    GLuint vertexObject = 0;
-    glGenBuffers(1, &vertexObject);
-    ASSERT_NE(vertexObject, 0U);
-    ASSERT_GL_NO_ERROR();
+    GLBuffer vertexObject;
 
-    glBindBuffer(GL_ARRAY_BUFFER, vertexObject);
+    glBindBuffer(GL_ARRAY_BUFFER, vertexObject.get());
     ASSERT_GL_NO_ERROR();
 
     GLfloat thePoints[] = {-0.5f + pixelOffset, -0.5f + pixelOffset, 0.5f + pixelOffset,
@@ -111,8 +107,6 @@ TEST_P(PointSpritesTest, PointCoordAndPointSizeCompliance)
     glDrawArrays(GL_POINTS, 0, 4);
     ASSERT_GL_NO_ERROR();
 
-    glDeleteBuffers(1, &vertexObject);
-
     std::string debugText;
     for (float py = 0; py < 2; ++py)
     {
@@ -182,8 +176,7 @@ TEST_P(PointSpritesTest, PointWithoutAttributesCompliance)
     );
     // clang-format on
 
-    GLuint program = CompileProgram(vs, fs);
-    ASSERT_NE(program, 0u);
+    ANGLE_GL_PROGRAM(program, vs, fs);
     ASSERT_GL_NO_ERROR();
 
     glUseProgram(program);
@@ -229,8 +222,7 @@ TEST_P(PointSpritesTest, PointCoordRegressionTest)
                           v_color = vec4(0.0, 1.0, 0.0, 1.0);
                       });
 
-    GLuint program = CompileProgram(vs, fs);
-    ASSERT_NE(program, 0u);
+    ANGLE_GL_PROGRAM(program, vs, fs);
     ASSERT_GL_NO_ERROR();
 
     glUseProgram(program);
@@ -253,12 +245,10 @@ TEST_P(PointSpritesTest, PointCoordRegressionTest)
     glUniform1f(pointSizeLoc, pointSize);
     ASSERT_GL_NO_ERROR();
 
-    GLuint vertexObject = 0;
-    glGenBuffers(1, &vertexObject);
-    ASSERT_NE(vertexObject, 0U);
+    GLBuffer vertexObject;
     ASSERT_GL_NO_ERROR();
 
-    glBindBuffer(GL_ARRAY_BUFFER, vertexObject);
+    glBindBuffer(GL_ARRAY_BUFFER, vertexObject.get());
     ASSERT_GL_NO_ERROR();
 
     GLfloat thePoints[] = {0.0f, 0.0f};
@@ -274,8 +264,6 @@ TEST_P(PointSpritesTest, PointCoordRegressionTest)
 
     // expect the center pixel to be green
     EXPECT_PIXEL_EQ((windowWidth - 1) / 2, (windowHeight - 1) / 2, 0, 255, 0, 255);
-
-    glDeleteBuffers(1, &vertexObject);
 }
 
 // Verify GL_VERTEX_PROGRAM_POINT_SIZE is enabled
@@ -306,8 +294,7 @@ TEST_P(PointSpritesTest, PointSizeEnabledCompliance)
     // The WebGL test is drawn on a 2x2 canvas. Emulate this by setting a 2x2 viewport.
     glViewport(0, 0, 2, 2);
 
-    GLuint program = CompileProgram(vs, fs);
-    ASSERT_NE(program, 0u);
+    ANGLE_GL_PROGRAM(program, vs, fs);
     ASSERT_GL_NO_ERROR();
 
     glUseProgram(program);
@@ -320,12 +307,10 @@ TEST_P(PointSpritesTest, PointSizeEnabledCompliance)
     GLfloat vertices[] = {0.4f, 0.4f, 0.0f};
     GLubyte colors[]   = {255, 0, 0, 255};
 
-    GLuint vertexObject = 0;
-    glGenBuffers(1, &vertexObject);
-    ASSERT_NE(vertexObject, 0U);
+    GLBuffer vertexObject;
     ASSERT_GL_NO_ERROR();
 
-    glBindBuffer(GL_ARRAY_BUFFER, vertexObject);
+    glBindBuffer(GL_ARRAY_BUFFER, vertexObject.get());
     ASSERT_GL_NO_ERROR();
 
     glBufferData(GL_ARRAY_BUFFER, sizeof(vertices) + sizeof(colors), nullptr, GL_STATIC_DRAW);
@@ -397,8 +382,6 @@ TEST_P(PointSpritesTest, PointSizeEnabledCompliance)
             }
         }
     }
-
-    glDeleteBuffers(1, &vertexObject);
 }
 
 // Verify that rendering works correctly when gl_PointSize is declared in a shader but isn't used
@@ -414,8 +397,7 @@ TEST_P(PointSpritesTest, PointSizeDeclaredButUnused)
     const std::string fs =
         SHADER_SOURCE(void main(void) { gl_FragColor = vec4(1.0, 0.0, 0.0, 1.0); });
 
-    GLuint program = CompileProgram(vs, fs);
-    ASSERT_NE(program, 0u);
+    ANGLE_GL_PROGRAM(program, vs, fs);
     ASSERT_GL_NO_ERROR();
 
     glUseProgram(program);
@@ -424,8 +406,6 @@ TEST_P(PointSpritesTest, PointSizeDeclaredButUnused)
 
     // expect the center pixel to be red
     EXPECT_PIXEL_EQ(getWindowWidth() / 2, getWindowHeight() / 2, 255, 0, 0, 255);
-
-    glDeleteProgram(program);
 }
 
 // Test to cover a bug where the D3D11 rasterizer state would not be update when switching between
@@ -472,12 +452,9 @@ TEST_P(PointSpritesTest, PointSpriteAlternatingDrawTypes)
     );
     // clang-format on
 
-    GLuint pointProgram = CompileProgram(pointVS, pointFS);
-    ASSERT_NE(pointProgram, 0u);
-    ASSERT_GL_NO_ERROR();
+    ANGLE_GL_PROGRAM(pointProgram, pointVS, pointFS);
 
-    GLuint quadProgram = CompileProgram(quadVS, quadFS);
-    ASSERT_NE(pointProgram, 0u);
+    ANGLE_GL_PROGRAM(quadProgram, quadVS, quadFS);
     ASSERT_GL_NO_ERROR();
 
     glEnable(GL_CULL_FACE);
@@ -501,9 +478,88 @@ TEST_P(PointSpritesTest, PointSpriteAlternatingDrawTypes)
 
     // expect the center pixel to be green
     EXPECT_PIXEL_EQ(getWindowWidth() / 2, getWindowHeight() / 2, 0, 255, 0, 255);
+}
+
+// This checks for an NVIDIA driver bug where points larger than the maximum reported point size can
+// be drawn. Point size should be clamped to the point size range as specified in GLES 3.0.5 section
+// 3.4.
+TEST_P(PointSpritesTest, PointSizeAboveMaxIsClamped)
+{
+    if (IsD3D9())
+    {
+        // Failed on NVIDIA GeForce GTX 1080 - no pixels from the point were detected in the
+        // framebuffer. http://anglebug.com/2111
+        std::cout << "Test skipped on D3D9." << std::endl;
+        return;
+    }
+
+    if (IsAMD() && IsOpenGL())
+    {
+        // Failed on AMD OSX and Windows trybots - no pixels from the point were detected in the
+        // framebuffer. http://anglebug.com/2113
+        std::cout << "Test skipped on AMD OpenGL." << std::endl;
+        return;
+    }
+
+    GLfloat pointSizeRange[2] = {};
+    glGetFloatv(GL_ALIASED_POINT_SIZE_RANGE, pointSizeRange);
+    GLfloat maxPointSize = pointSizeRange[1];
+
+    if (maxPointSize < 4)
+    {
+        // This test is only able to test larger points.
+        return;
+    }
+
+    const std::string &vs =
+        "attribute vec4 vPosition;\n"
+        "uniform float uPointSize;\n"
+        "void main()\n"
+        "{\n"
+        "    gl_PointSize = uPointSize;\n"
+        "    gl_Position  = vPosition;\n"
+        "}\n";
+    const std::string &fs =
+        "precision mediump float;\n"
+        "void main()\n"
+        "{\n"
+        "    gl_FragColor = vec4(1, 0, 0, 1);\n"
+        "}\n";
+    ANGLE_GL_PROGRAM(program, vs, fs);
+    glUseProgram(program);
+    ASSERT_GL_NO_ERROR();
+
+    GLfloat testPointSize = floorf(maxPointSize * 2.0f);
+
+    GLint pointSizeLoc = glGetUniformLocation(program, "uPointSize");
+    glUniform1f(pointSizeLoc, testPointSize);
+    ASSERT_GL_NO_ERROR();
+
+    // The point will be a square centered at gl_Position. We'll offset it from the center of the
+    // viewport on the x axis so that the left edge of the point square is at the center of the
+    // viewport.
+    GLfloat pointXPosition = (0.5f * maxPointSize) * (2.0f / (GLfloat)getWindowWidth());
+
+    GLBuffer vertexBuffer;
+    glBindBuffer(GL_ARRAY_BUFFER, vertexBuffer.get());
+    GLfloat thePoints[] = {pointXPosition, 0.0f};
+    glBufferData(GL_ARRAY_BUFFER, sizeof(thePoints), thePoints, GL_STATIC_DRAW);
+    ASSERT_GL_NO_ERROR();
+
+    glEnableVertexAttribArray(0);
+    glVertexAttribPointer(0, 2, GL_FLOAT, GL_FALSE, 0, 0);
+
+    glClearColor(0, 0, 0, 0);
+    glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
+
+    glDrawArrays(GL_POINTS, 0, 1);
+    ASSERT_GL_NO_ERROR();
+
+    // Pixel on the right of the viewport center should be covered by the point.
+    EXPECT_PIXEL_NEAR(getWindowWidth() / 2 + 2, getWindowHeight() / 2, 255, 0, 0, 255, 4);
 
-    glDeleteProgram(pointProgram);
-    glDeleteProgram(quadProgram);
+    // Pixel on the left of the viewport center should not be covered by the point.
+    EXPECT_PIXEL_NEAR(getWindowWidth() / 2 - 2, getWindowHeight() / 2, 0, 0, 0, 0, 4);
 }
 
 // Use this to select which configurations (e.g. which renderer, which GLES