Reland 'Remove IndexRange retrieving in validation'

src/libANGLE/Caps.cpp

@@ -177,6 +177,7 @@ Extensions::Extensions()
       queryCounterBitsTimeElapsed(0),
       queryCounterBitsTimestamp(0),
       robustness(false),
+      robustBufferAccessBehavior(false),
       blendMinMax(false),
       framebufferBlit(false),
       framebufferMultisample(false),
@@ -651,6 +652,7 @@ const ExtensionInfoMap &GetExtensionInfoMap()
         map["GL_ANGLE_timer_query"] = esOnlyExtension(&Extensions::timerQuery);
         map["GL_EXT_disjoint_timer_query"] = esOnlyExtension(&Extensions::disjointTimerQuery);
         map["GL_EXT_robustness"] = esOnlyExtension(&Extensions::robustness);
+        map["GL_KHR_robust_buffer_access_behavior"] = esOnlyExtension(&Extensions::robustBufferAccessBehavior);
         map["GL_EXT_blend_minmax"] = esOnlyExtension(&Extensions::blendMinMax);
         map["GL_ANGLE_framebuffer_blit"] = esOnlyExtension(&Extensions::framebufferBlit);
         map["GL_ANGLE_framebuffer_multisample"] = esOnlyExtension(&Extensions::framebufferMultisample);

src/libANGLE/Caps.h

@@ -218,6 +218,9 @@ struct Extensions
     // GL_EXT_robustness
     bool robustness;
 
+    // GL_KHR_robust_buffer_access_behavior
+    bool robustBufferAccessBehavior;
+
     // GL_EXT_blend_minmax
     bool blendMinMax;
 

src/libANGLE/Context.cpp

@@ -280,11 +280,6 @@ Context::Context(rx::EGLImplFactory *implFactory,
       mScratchBuffer(1000u),
       mZeroFilledBuffer(1000u)
 {
-    if (mRobustAccess)
-    {
-        UNIMPLEMENTED();
-    }
-
     initCaps(displayExtensions);
     initWorkarounds();
 
@@ -2610,6 +2605,11 @@ void Context::initCaps(const egl::DisplayExtensions &displayExtensions)
     mExtensions.robustResourceInitialization =
         egl::Display::GetClientExtensions().displayRobustResourceInitialization;
 
+    // mExtensions.robustBufferAccessBehavior is true only if robust access is true and the backend
+    // supports it.
+    mExtensions.robustBufferAccessBehavior =
+        mRobustAccess && mExtensions.robustBufferAccessBehavior;
+
     // Enable the cache control query unconditionally.
     mExtensions.programCacheControl = true;
 

src/libANGLE/renderer/d3d/VertexDataManager.cpp

@@ -410,7 +410,7 @@ gl::Error VertexDataManager::storeDynamicAttribs(
     for (auto attribIndex : dynamicAttribsMask)
     {
         const auto &dynamicAttrib = (*translatedAttribs)[attribIndex];
-        ANGLE_TRY(reserveSpaceForAttrib(dynamicAttrib, count, instances));
+        ANGLE_TRY(reserveSpaceForAttrib(dynamicAttrib, start, count, instances));
     }
 
     // Store dynamic attributes
@@ -445,6 +445,7 @@ void VertexDataManager::PromoteDynamicAttribs(
 }
 
 gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
+                                                   GLint start,
                                                    GLsizei count,
                                                    GLsizei instances) const
 {
@@ -460,10 +461,23 @@ gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &tr
 
     size_t totalCount = gl::ComputeVertexBindingElementCount(
         binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances));
-    ASSERT(!bufferD3D ||
-           ElementsInBuffer(attrib, binding, static_cast<unsigned int>(bufferD3D->getSize())) >=
-               static_cast<int>(totalCount));
+    // TODO(jiajia.qin@intel.com): force the index buffer to clamp any out of range indices instead
+    // of invalid operation here.
+    if (bufferD3D)
+    {
+        // Vertices do not apply the 'start' offset when the divisor is non-zero even when doing
+        // a non-instanced draw call
+        GLint firstVertexIndex = binding.getDivisor() > 0 ? 0 : start;
+        int64_t maxVertexCount =
+            static_cast<int64_t>(firstVertexIndex) + static_cast<int64_t>(totalCount);
+        int elementsInBuffer =
+            ElementsInBuffer(attrib, binding, static_cast<unsigned int>(bufferD3D->getSize()));
 
+        if (maxVertexCount > elementsInBuffer)
+        {
+            return gl::InvalidOperation() << "Vertex buffer is not big enough for the draw call.";
+        }
+    }
     return mStreamingBuffer->reserveVertexSpace(attrib, binding, static_cast<GLsizei>(totalCount),
                                                 instances);
 }

src/libANGLE/renderer/d3d/VertexDataManager.h

@@ -128,6 +128,7 @@ class VertexDataManager : angle::NonCopyable
 
     gl::Error reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
                                     GLsizei count,
+                                    GLint start,
                                     GLsizei instances) const;
 
     gl::Error storeDynamicAttrib(TranslatedAttribute *translated,

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

@@ -1381,6 +1381,9 @@ void GenerateCaps(ID3D11Device *device, ID3D11DeviceContext *deviceContext, cons
     extensions->queryCounterBitsTimestamp =
         0;  // Timestamps cannot be supported due to D3D11 limitations
     extensions->robustness = true;
+    // Direct3D guarantees to return zero for any resource that is accessed out of bounds.
+    // See https://msdn.microsoft.com/en-us/library/windows/desktop/ff476332(v=vs.85).aspx
+    extensions->robustBufferAccessBehavior = true;
     extensions->blendMinMax = true;
     extensions->framebufferBlit = GetFramebufferBlitSupport(featureLevel);
     extensions->framebufferMultisample = GetFramebufferMultisampleSupport(featureLevel);

src/libANGLE/renderer/d3d/d3d9/renderer9_utils.cpp

@@ -579,6 +579,9 @@ void GenerateCaps(IDirect3D9 *d3d9,
     extensions->timerQuery = false; // Unimplemented
     extensions->disjointTimerQuery     = false;
     extensions->robustness = true;
+    // Direct3D guarantees to return zero for any resource that is accessed out of bounds.
+    // See https://msdn.microsoft.com/en-us/library/windows/desktop/ff476332(v=vs.85).aspx
+    extensions->robustBufferAccessBehavior = true;
     extensions->blendMinMax = true;
     extensions->framebufferBlit = true;
     extensions->framebufferMultisample = true;

src/libANGLE/renderer/gl/glx/DisplayGLX.cpp

@@ -821,6 +821,8 @@ egl::Error DisplayGLX::createContextAttribs(glx::FBConfig,
 
     if (mHasARBCreateContextRobustness)
     {
+        attribs.push_back(GLX_CONTEXT_FLAGS_ARB);
+        attribs.push_back(GLX_CONTEXT_ROBUST_ACCESS_BIT_ARB);
         attribs.push_back(GLX_CONTEXT_RESET_NOTIFICATION_STRATEGY_ARB);
         attribs.push_back(GLX_LOSE_CONTEXT_ON_RESET_ARB);
     }

src/libANGLE/renderer/gl/renderergl_utils.cpp

@@ -953,6 +953,11 @@ void GenerateCaps(const FunctionsGL *functions,
                              functions->hasGLESExtension("GL_KHR_robustness") ||
                              functions->hasGLESExtension("GL_EXT_robustness");
 
+    extensions->robustBufferAccessBehavior =
+        extensions->robustness &&
+        (functions->hasGLExtension("GL_ARB_robust_buffer_access_behavior") ||
+         functions->hasGLESExtension("GL_KHR_robust_buffer_access_behavior"));
+
     extensions->copyTexture = true;
     extensions->syncQuery   = SyncQueryGL::IsSupported(functions);
 

src/libANGLE/renderer/gl/wgl/DisplayWGL.cpp

@@ -773,6 +773,8 @@ HGLRC DisplayWGL::createContextAttribs(const gl::Version &version, int profileMa
 
     if (mHasWGLCreateContextRobustness)
     {
+        attribs.push_back(WGL_CONTEXT_FLAGS_ARB);
+        attribs.push_back(WGL_CONTEXT_ROBUST_ACCESS_BIT_ARB);
         attribs.push_back(WGL_CONTEXT_RESET_NOTIFICATION_STRATEGY_ARB);
         attribs.push_back(WGL_LOSE_CONTEXT_ON_RESET_ARB);
     }

src/libANGLE/validationES.cpp

@@ -2887,34 +2887,46 @@ bool ValidateDrawElementsCommon(ValidationContext *context,
         }
     }
 
-    // Use the parameter buffer to retrieve and cache the index range.
-    // TODO: offer fast path, with disabled index validation.
-    // TODO: also disable index checking on back-ends that are robust to out-of-range accesses.
-    const auto &params        = context->getParams<HasIndexRange>();
-    const auto &indexRangeOpt = params.getIndexRange();
-    if (!indexRangeOpt.valid())
+    if (context->getExtensions().robustBufferAccessBehavior)
     {
-        // Unexpected error.
-        return false;
+        // Here we use maxVertex = 0 and vertexCount = 1 to avoid retrieving IndexRange when robust
+        // access is enabled.
+        if (!ValidateDrawAttribs(context, primcount, 0, 1))
+        {
+            return false;
+        }
     }
-
-    // If we use an index greater than our maximum supported index range, return an error.
-    // The ES3 spec does not specify behaviour here, it is undefined, but ANGLE should always
-    // return an error if possible here.
-    if (static_cast<GLuint64>(indexRangeOpt.value().end) >= context->getCaps().maxElementIndex)
+    else
     {
-        ANGLE_VALIDATION_ERR(context, InvalidOperation(), ExceedsMaxElement);
-        return false;
-    }
+        // Use the parameter buffer to retrieve and cache the index range.
+        const auto &params        = context->getParams<HasIndexRange>();
+        const auto &indexRangeOpt = params.getIndexRange();
+        if (!indexRangeOpt.valid())
+        {
+            // Unexpected error.
+            return false;
+        }
 
-    if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(indexRangeOpt.value().end),
-                             static_cast<GLint>(indexRangeOpt.value().vertexCount())))
-    {
-        return false;
+        // If we use an index greater than our maximum supported index range, return an error.
+        // The ES3 spec does not specify behaviour here, it is undefined, but ANGLE should always
+        // return an error if possible here.
+        if (static_cast<GLuint64>(indexRangeOpt.value().end) >= context->getCaps().maxElementIndex)
+        {
+            ANGLE_VALIDATION_ERR(context, InvalidOperation(), ExceedsMaxElement);
+            return false;
+        }
+
+        if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(indexRangeOpt.value().end),
+                                 static_cast<GLint>(indexRangeOpt.value().vertexCount())))
+        {
+            return false;
+        }
+
+        // No op if there are no real indices in the index data (all are primitive restart).
+        return (indexRangeOpt.value().vertexIndexCount > 0);
     }
 
-    // No op if there are no real indices in the index data (all are primitive restart).
-    return (indexRangeOpt.value().vertexIndexCount > 0);
+    return true;
 }
 
 bool ValidateDrawElementsInstancedCommon(ValidationContext *context,

src/libANGLE/validationES_unittest.cpp

@@ -56,10 +56,10 @@ class MockValidationContext : public ValidationContext
 };
 
 // Test that ANGLE generates an INVALID_OPERATION when validating index data that uses a value
-// larger than MAX_ELEMENT_INDEX. Not specified in the GLES 3 spec, it's undefined behaviour,
-// but we want a test to ensure we maintain this behaviour.
-// TODO(jmadill): Re-enable when framebuffer sync state doesn't happen in validation.
-// Also broken because of change of api of the state initialize method.
+// larger than MAX_ELEMENT_INDEX and robust access is not enabled. Not specified in the GLES 3 spec,
+// it's undefined behaviour, but we want a test to ensure we maintain this behaviour. TODO(jmadill):
+// Re-enable when framebuffer sync state doesn't happen in validation. Also broken because of change
+// of api of the state initialize method.
 TEST(ValidationESTest, DISABLED_DrawElementsWithMaxIndexGivesError)
 {
     auto framebufferImpl = MakeFramebufferMock();
@@ -116,8 +116,11 @@ TEST(ValidationESTest, DISABLED_DrawElementsWithMaxIndexGivesError)
                           3, 4, static_cast<GLuint>(caps.maxElementIndex)};
     EXPECT_TRUE(
         ValidateDrawElementsCommon(&testContext, GL_TRIANGLES, 3, GL_UNSIGNED_INT, indexData, 1));
-    EXPECT_FALSE(
-        ValidateDrawElementsCommon(&testContext, GL_TRIANGLES, 6, GL_UNSIGNED_INT, indexData, 2));
+    if (!testContext.getExtensions().robustBufferAccessBehavior)
+    {
+        EXPECT_FALSE(ValidateDrawElementsCommon(&testContext, GL_TRIANGLES, 6, GL_UNSIGNED_INT,
+                                                indexData, 2));
+    }
 
     texture->release(nullptr);
 

src/tests/gl_tests/VertexAttributeTest.cpp

@@ -785,6 +785,13 @@ TEST_P(VertexAttributeTest, DrawArraysBufferTooSmall)
 // Verify that index draw with an out-of-range offset generates INVALID_OPERATION.
 TEST_P(VertexAttributeTest, DrawElementsBufferTooSmall)
 {
+    if (extensionEnabled("GL_KHR_robust_buffer_access_behavior"))
+    {
+        std::cout << "Test skipped due to supporting GL_KHR_robust_buffer_access_behavior"
+                  << std::endl;
+        return;
+    }
+
     std::array<GLfloat, kVertexCount> inputData;
     std::array<GLfloat, kVertexCount> expectedData;
     InitTestData(inputData, expectedData);

src/tests/test_utils/ANGLETest.cpp

@@ -819,6 +819,11 @@ void ANGLETestBase::setWebGLCompatibilityEnabled(bool webglCompatibility)
     mEGLWindow->setWebGLCompatibilityEnabled(webglCompatibility);
 }
 
+void ANGLETestBase::setRobustAccess(bool enabled)
+{
+    mEGLWindow->setRobustAccess(enabled);
+}
+
 void ANGLETestBase::setBindGeneratesResource(bool bindGeneratesResource)
 {
     mEGLWindow->setBindGeneratesResource(bindGeneratesResource);

src/tests/test_utils/ANGLETest.h

@@ -303,6 +303,7 @@ class ANGLETestBase
     void setDebugEnabled(bool enabled);
     void setNoErrorEnabled(bool enabled);
     void setWebGLCompatibilityEnabled(bool webglCompatibility);
+    void setRobustAccess(bool enabled);
     void setBindGeneratesResource(bool bindGeneratesResource);
     void setDebugLayersEnabled(bool enabled);
     void setClientArraysEnabled(bool enabled);

util/EGLWindow.cpp

@@ -119,6 +119,7 @@ EGLWindow::EGLWindow(EGLint glesMajorVersion,
       mWebGLCompatibility(false),
       mBindGeneratesResource(true),
       mClientArraysEnabled(true),
+      mRobustAccess(false),
       mRobustResourceInit(),
       mSwapInterval(-1),
       mSamples(-1),
@@ -323,6 +324,13 @@ bool EGLWindow::initializeContext()
         return false;
     }
 
+    bool hasRobustness = strstr(displayExtensions, "EGL_EXT_create_context_robustness") != nullptr;
+    if (mRobustAccess && !hasRobustness)
+    {
+        destroyGL();
+        return false;
+    }
+
     bool hasBindGeneratesResource =
         strstr(displayExtensions, "EGL_CHROMIUM_create_context_bind_generates_resource") != nullptr;
     if (!mBindGeneratesResource && !hasBindGeneratesResource)
@@ -380,6 +388,12 @@ bool EGLWindow::initializeContext()
             contextAttributes.push_back(mWebGLCompatibility ? EGL_TRUE : EGL_FALSE);
         }
 
+        if (hasRobustness)
+        {
+            contextAttributes.push_back(EGL_CONTEXT_OPENGL_ROBUST_ACCESS_EXT);
+            contextAttributes.push_back(mRobustAccess ? EGL_TRUE : EGL_FALSE);
+        }
+
         if (hasBindGeneratesResource)
         {
             contextAttributes.push_back(EGL_CONTEXT_BIND_GENERATES_RESOURCE_CHROMIUM);

util/EGLWindow.h

@@ -86,6 +86,7 @@ class ANGLE_EXPORT EGLWindow : angle::NonCopyable
     }
     void setDebugLayersEnabled(bool enabled) { mDebugLayersEnabled = enabled; }
     void setClientArraysEnabled(bool enabled) { mClientArraysEnabled = enabled; }
+    void setRobustAccess(bool enabled) { mRobustAccess = enabled; }
     void setRobustResourceInit(bool enabled) { mRobustResourceInit = enabled; }
     void setSwapInterval(EGLint swapInterval) { mSwapInterval = swapInterval; }
     void setPlatformMethods(angle::PlatformMethods *platformMethods)
@@ -156,6 +157,7 @@ class ANGLE_EXPORT EGLWindow : angle::NonCopyable
     bool mWebGLCompatibility;
     bool mBindGeneratesResource;
     bool mClientArraysEnabled;
+    bool mRobustAccess;
     Optional<bool> mRobustResourceInit;
     EGLint mSwapInterval;
     EGLint mSamples;