Bug 1324972 (flattened) - Disable unnecessary manual index validation for WebGL 2. - r=daoshengmu
authorJeff Gilbert <jgilbert@mozilla.com>
Tue, 20 Dec 2016 18:44:28 -0800
changeset 327536 eb4b2c6559b6e8fcc7ad61b62a127559d0694931
parent 327535 1882ac4adbc561063b4c527769583d102bd93f42
child 327537 82b7028a88c7833e044b22c8975fda1eeeb23673
push id35517
push userkwierso@gmail.com
push dateThu, 29 Dec 2016 20:22:54 +0000
treeherderautoland@3f2f8d77ad27 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdaoshengmu
bugs1324972
milestone53.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1324972 (flattened) - Disable unnecessary manual index validation for WebGL 2. - r=daoshengmu Includes: * Catch ANGLE's zealous index-out-of-bounds INVALID_OPs. * ANGLE DrawElements validation is wrong.
dom/canvas/WebGLBuffer.cpp
dom/canvas/WebGLContextDraw.cpp
dom/canvas/test/webgl-conf/generated-mochitest.ini
dom/canvas/test/webgl-conf/mochitest-errata.ini
gfx/angle/src/libANGLE/validationES.cpp
--- a/dom/canvas/WebGLBuffer.cpp
+++ b/dom/canvas/WebGLBuffer.cpp
@@ -168,50 +168,64 @@ WebGLBuffer::ValidateRange(const char* f
 }
 
 ////////////////////////////////////////
 
 bool
 WebGLBuffer::ElementArrayCacheBufferData(const void* ptr,
                                          size_t bufferSizeInBytes)
 {
+    if (mContext->IsWebGL2())
+        return true;
+
     if (mContent == Kind::ElementArray)
         return mCache->BufferData(ptr, bufferSizeInBytes);
 
     return true;
 }
 
 void
 WebGLBuffer::ElementArrayCacheBufferSubData(size_t pos, const void* ptr,
                                             size_t updateSizeInBytes)
 {
+    if (mContext->IsWebGL2())
+        return;
+
     if (mContent == Kind::ElementArray)
         mCache->BufferSubData(pos, ptr, updateSizeInBytes);
 }
 
 size_t
 WebGLBuffer::SizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf) const
 {
     size_t sizeOfCache = mCache ? mCache->SizeOfIncludingThis(mallocSizeOf)
                                 : 0;
     return mallocSizeOf(this) + sizeOfCache;
 }
 
 bool
 WebGLBuffer::Validate(GLenum type, uint32_t maxAllowed, size_t first, size_t count) const
 {
+    if (mContext->IsWebGL2())
+        return true;
+
     return mCache->Validate(type, maxAllowed, first, count);
 }
 
 bool
 WebGLBuffer::IsElementArrayUsedWithMultipleTypes() const
 {
+    if (mContext->IsWebGL2())
+        return false;
+
     return mCache->BeenUsedWithMultipleTypes();
 }
 
+////
+
 bool
 WebGLBuffer::ValidateCanBindToTarget(const char* funcName, GLenum target)
 {
     /* https://www.khronos.org/registry/webgl/specs/latest/2.0/#5.1
      *
      * In the WebGL 2 API, buffers have their WebGL buffer type
      * initially set to undefined. Calling bindBuffer, bindBufferRange
      * or bindBufferBase with the target argument set to any buffer
--- a/dom/canvas/WebGLContextDraw.cpp
+++ b/dom/canvas/WebGLContextDraw.cpp
@@ -689,16 +689,36 @@ WebGLContext::DrawElements_check(const c
         GenerateWarning("%s: bound element array buffer previously used with a type other than "
                         "%s, this will affect performance.",
                         funcName, WebGLContext::EnumName(type));
     }
 
     return true;
 }
 
+static void
+HandleDrawElementsErrors(WebGLContext* webgl, const char* funcName,
+                         gl::GLContext::LocalErrorScope& errorScope)
+{
+    const auto err = errorScope.GetError();
+    if (err == LOCAL_GL_INVALID_OPERATION) {
+        webgl->ErrorInvalidOperation("%s: Driver rejected indexed draw call, possibly"
+                                     " due to out-of-bounds indices.", funcName);
+        return;
+    }
+
+    MOZ_ASSERT(!err);
+    if (err) {
+        webgl->ErrorImplementationBug("%s: Unexpected driver error during indexed draw"
+                                      " call. Please file a bug.",
+                                      funcName);
+        return;
+    }
+}
+
 void
 WebGLContext::DrawElements(GLenum mode, GLsizei vertCount, GLenum type,
                            WebGLintptr byteOffset, const char* funcName)
 {
     if (!funcName) {
         funcName = "drawElements";
     }
 
@@ -718,18 +738,30 @@ WebGLContext::DrawElements(GLenum mode, 
 
     const ScopedDrawHelper scopedHelper(this, funcName, 0, mMaxFetchedVertices, instanceCount,
                                         &error);
     if (error)
         return;
 
     {
         ScopedDrawCallWrapper wrapper(*this);
-        gl->fDrawElements(mode, vertCount, type,
-                          reinterpret_cast<GLvoid*>(byteOffset));
+        {
+            UniquePtr<gl::GLContext::LocalErrorScope> errorScope;
+
+            if (gl->IsANGLE()) {
+                errorScope.reset(new gl::GLContext::LocalErrorScope(*gl));
+            }
+
+            gl->fDrawElements(mode, vertCount, type,
+                              reinterpret_cast<GLvoid*>(byteOffset));
+
+            if (errorScope) {
+                HandleDrawElementsErrors(this, funcName, *errorScope);
+            }
+        }
     }
 
     Draw_cleanup(funcName);
 }
 
 void
 WebGLContext::DrawElementsInstanced(GLenum mode, GLsizei vertCount, GLenum type,
                                     WebGLintptr byteOffset, GLsizei instanceCount)
@@ -753,19 +785,30 @@ WebGLContext::DrawElementsInstanced(GLen
 
     const ScopedDrawHelper scopedHelper(this, funcName, 0, mMaxFetchedVertices, instanceCount,
                                         &error);
     if (error)
         return;
 
     {
         ScopedDrawCallWrapper wrapper(*this);
-        gl->fDrawElementsInstanced(mode, vertCount, type,
-                                   reinterpret_cast<GLvoid*>(byteOffset),
-                                   instanceCount);
+        {
+            UniquePtr<gl::GLContext::LocalErrorScope> errorScope;
+
+            if (gl->IsANGLE()) {
+                errorScope.reset(new gl::GLContext::LocalErrorScope(*gl));
+            }
+
+            gl->fDrawElementsInstanced(mode, vertCount, type,
+                                       reinterpret_cast<GLvoid*>(byteOffset),
+                                       instanceCount);
+            if (errorScope) {
+                HandleDrawElementsErrors(this, funcName, *errorScope);
+            }
+        }
     }
 
     Draw_cleanup(funcName);
 }
 
 ////////////////////////////////////////
 
 void
--- a/dom/canvas/test/webgl-conf/generated-mochitest.ini
+++ b/dom/canvas/test/webgl-conf/generated-mochitest.ini
@@ -4569,16 +4569,17 @@ skip-if = (os == 'android' || os == 'lin
 [generated/test_2_conformance2__renderbuffers__multisampled-renderbuffer-initialization.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance2__renderbuffers__readbuffer.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance2__rendering__draw-buffers.html]
 fail-if = (os == 'mac') || (os == 'win')
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance2__rendering__element-index-uint.html]
+fail-if = (os != 'win')
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance2__rendering__framebuffer-completeness-unaffected.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance2__rendering__instanced-arrays.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance2__samplers__sampler-drawing-test.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance2__samplers__samplers.html]
@@ -4621,26 +4622,29 @@ skip-if = (os == 'android' || os == 'lin
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__buffers__buffer-data-and-buffer-sub-data.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__buffers__buffer-data-array-buffer-delete.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__buffers__element-array-buffer-delete-recreate.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__buffers__index-validation-copies-indices.html]
+fail-if = (os != 'win')
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__buffers__index-validation-crash-with-buffer-sub-data.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__buffers__index-validation-large-buffer.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__buffers__index-validation-verifies-too-many-indices.html]
+fail-if = (os != 'win')
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__buffers__index-validation-with-resized-buffer.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__buffers__index-validation.html]
+fail-if = (os != 'win')
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__canvas__buffer-offscreen-test.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__canvas__buffer-preserve-test.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__canvas__canvas-test.html]
 skip-if = (os == 'win') || (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__canvas__canvas-zero-size.html]
@@ -5738,16 +5742,17 @@ skip-if = (os == 'android' || os == 'lin
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__rendering__culling.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__rendering__default-texture-draw-bug.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__rendering__draw-arrays-out-of-bounds.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__rendering__draw-elements-out-of-bounds.html]
+fail-if = (os != 'win')
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__rendering__draw-with-changing-start-vertex-bug.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__rendering__framebuffer-switch.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__rendering__framebuffer-texture-switch.html]
 skip-if = (os == 'android' || os == 'linux' || (os == 'win' && os_version == '5.1'))
 [generated/test_2_conformance__rendering__gl-clear.html]
--- a/dom/canvas/test/webgl-conf/mochitest-errata.ini
+++ b/dom/canvas/test/webgl-conf/mochitest-errata.ini
@@ -76,16 +76,30 @@ skip-if = 1
 # Timing out
 [generated/test_conformance__uniforms__uniform-default-values.html]
 # Timeout on Windows, crash on Android/Linux.
 skip-if = (os == 'android') || (os == 'linux') || (os == 'win')
 [generated/test_conformance__ogles__GL__mat3__mat3_001_to_006.html]
 # Timeout on D3D11
 skip-if = (os == 'win' && os_version != '5.1')
 
+####################
+# Tests expect conservative index validation, which we skip on WebGL 2.
+# ANGLE still provides it though, so they pass on windows.
+[generated/test_2_conformance__rendering__draw-elements-out-of-bounds.html]
+fail-if = (os != 'win')
+[generated/test_2_conformance__buffers__index-validation-copies-indices.html]
+fail-if = (os != 'win')
+[generated/test_2_conformance__buffers__index-validation.html]
+fail-if = (os != 'win')
+[generated/test_2_conformance__buffers__index-validation-verifies-too-many-indices.html]
+fail-if = (os != 'win')
+[generated/test_2_conformance2__rendering__element-index-uint.html]
+fail-if = (os != 'win')
+
 ########################################################################
 # Complicated
 
 [generated/test_conformance__context__context-attributes-alpha-depth-stencil-antialias.html]
 fail-if = (os == 'mac' && os_version == '10.6')
 # Asserts on 'B2G ICS Emulator Debug' and linux debug. Crashes on Android.
 skip-if = (os == 'b2g') || (os == 'linux') || (os == 'android')
 
--- a/gfx/angle/src/libANGLE/validationES.cpp
+++ b/gfx/angle/src/libANGLE/validationES.cpp
@@ -3214,17 +3214,17 @@ bool ValidateDrawElements(ValidationCont
     // 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>(indexRangeOut->end) >= context->getCaps().maxElementIndex)
     {
         context->handleError(Error(GL_INVALID_OPERATION, g_ExceedsMaxElementErrorMessage));
         return false;
     }
 
-    if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(indexRangeOut->vertexCount())))
+    if (!ValidateDrawAttribs(context, primcount, static_cast<GLint>(indexRangeOut->end + 1)))
     {
         return false;
     }
 
     // No op if there are no real indices in the index data (all are primitive restart).
     return (indexRangeOut->vertexIndexCount > 0);
 }