Bug 1563076 - Forbid BeginTransformFeedback with a buffer bound to multiple indices. r=lsalzman
authorJeff Gilbert <jgilbert@mozilla.com>
Wed, 03 Jul 2019 02:22:52 +0000
changeset 540708 e25e794f08228b95ef36e744695a09abda9c7440
parent 540707 3416303a9b4a7c49272e956f9a83395e4e4ad985
child 540709 a47da25dc19653b7a19d54119589dbc340c34420
push id11529
push userarchaeopteryx@coole-files.de
push dateThu, 04 Jul 2019 15:22:33 +0000
treeherdermozilla-beta@ebb510a784b8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslsalzman
bugs1563076
milestone69.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 1563076 - Forbid BeginTransformFeedback with a buffer bound to multiple indices. r=lsalzman Differential Revision: https://phabricator.services.mozilla.com/D36688
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextDraw.cpp
dom/canvas/WebGLTransformFeedback.cpp
dom/canvas/test/webgl-conf/generated-mochitest.ini
dom/canvas/test/webgl-conf/mochitest-errata.ini
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -1091,16 +1091,19 @@ class WebGLContext : public nsICanvasRen
   std::vector<IndexedBufferBinding> mIndexedUniformBufferBindings;
 
   WebGLRefPtr<WebGLBuffer>& GetBufferSlotByTarget(GLenum target);
   WebGLRefPtr<WebGLBuffer>& GetBufferSlotByTargetIndexed(GLenum target,
                                                          GLuint index);
 
   // -
 
+  void GenErrorIllegalUse(GLenum useTarget, uint32_t useId, GLenum boundTarget,
+                          uint32_t boundId) const;
+
   bool ValidateBufferForNonTf(const WebGLBuffer&, GLenum nonTfTarget,
                               uint32_t nonTfId) const;
 
   bool ValidateBufferForNonTf(const WebGLBuffer* const nonTfBuffer,
                               const GLenum nonTfTarget,
                               const uint32_t nonTfId = -1) const {
     if (!nonTfBuffer) return true;
     return ValidateBufferForNonTf(*nonTfBuffer, nonTfTarget, nonTfId);
--- a/dom/canvas/WebGLContextDraw.cpp
+++ b/dom/canvas/WebGLContextDraw.cpp
@@ -197,33 +197,33 @@ bool WebGLContext::ValidateStencilParams
         " [0, (2^s)-1], where `s` is the number of enabled stencil"
         " bits in the draw framebuffer)");
   }
   return ok;
 }
 
 // -
 
-static void GenErrorIllegalUse(const WebGLContext& webgl,
-                               const GLenum useTarget, const uint32_t useId,
-                               const GLenum boundTarget,
-                               const uint32_t boundId) {
+void WebGLContext::GenErrorIllegalUse(const GLenum useTarget,
+                                      const uint32_t useId,
+                                      const GLenum boundTarget,
+                                      const uint32_t boundId) const {
   const auto fnName = [&](const GLenum target, const uint32_t id) {
     auto name = nsCString(EnumString(target).c_str());
     if (id != static_cast<uint32_t>(-1)) {
       name += nsPrintfCString("[%u]", id);
     }
     return name;
   };
   const auto& useName = fnName(useTarget, useId);
   const auto& boundName = fnName(boundTarget, boundId);
-  webgl.GenerateError(LOCAL_GL_INVALID_OPERATION,
-                      "Illegal use of buffer at %s"
-                      " while also bound to %s.",
-                      useName.BeginReading(), boundName.BeginReading());
+  GenerateError(LOCAL_GL_INVALID_OPERATION,
+                "Illegal use of buffer at %s"
+                " while also bound to %s.",
+                useName.BeginReading(), boundName.BeginReading());
 }
 
 bool WebGLContext::ValidateBufferForNonTf(const WebGLBuffer& nonTfBuffer,
                                           const GLenum nonTfTarget,
                                           const uint32_t nonTfId) const {
   bool dupe = false;
   const auto& tfAttribs = mBoundTransformFeedback->mIndexedBindings;
   for (const auto& cur : tfAttribs) {
@@ -231,17 +231,17 @@ bool WebGLContext::ValidateBufferForNonT
   }
   if (MOZ_LIKELY(!dupe)) return true;
 
   dupe = false;
   for (const auto tfId : IntegerRange(tfAttribs.size())) {
     const auto& tfBuffer = tfAttribs[tfId].mBufferBinding;
     if (&nonTfBuffer == tfBuffer) {
       dupe = true;
-      GenErrorIllegalUse(*this, nonTfTarget, nonTfId,
+      GenErrorIllegalUse(nonTfTarget, nonTfId,
                          LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER, tfId);
     }
   }
   MOZ_ASSERT(dupe);
   return false;
 }
 
 bool WebGLContext::ValidateBuffersForTf(
@@ -280,17 +280,17 @@ bool WebGLContext::ValidateBuffersForTf(
       dupe |= (nonTf && tf.buffer == nonTf);
     }
 
     if (MOZ_LIKELY(!dupe)) return false;
 
     for (const auto& tf : tfBuffers) {
       if (nonTf && tf.buffer == nonTf) {
         dupe = true;
-        GenErrorIllegalUse(*this, LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER, tf.id,
+        GenErrorIllegalUse(LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER, tf.id,
                            nonTfTarget, nonTfId);
       }
     }
     return true;
   };
 
   fnCheck(mBoundArrayBuffer.get(), LOCAL_GL_ARRAY_BUFFER, -1);
   fnCheck(mBoundCopyReadBuffer.get(), LOCAL_GL_COPY_READ_BUFFER, -1);
--- a/dom/canvas/WebGLTransformFeedback.cpp
+++ b/dom/canvas/WebGLTransformFeedback.cpp
@@ -2,16 +2,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "WebGLTransformFeedback.h"
 
 #include "GLContext.h"
 #include "mozilla/dom/WebGL2RenderingContextBinding.h"
+#include "mozilla/IntegerRange.h"
 #include "WebGL2Context.h"
 #include "WebGLProgram.h"
 
 namespace mozilla {
 
 WebGLTransformFeedback::WebGLTransformFeedback(WebGLContext* webgl, GLuint tf)
     : WebGLRefCountedObject(webgl),
       mGLName(tf),
@@ -68,16 +69,26 @@ void WebGLTransformFeedback::BeginTransf
     if (!buffer) {
       mContext->ErrorInvalidOperation(
           "No buffer attached to required transform"
           " feedback index %u.",
           (uint32_t)i);
       return;
     }
 
+    for (const auto iBound : IntegerRange(mIndexedBindings.size())) {
+      const auto& bound = mIndexedBindings[iBound].mBufferBinding.get();
+      if (iBound != i && buffer == bound) {
+        mContext->GenErrorIllegalUse(
+            LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER, static_cast<uint32_t>(i),
+            LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER, static_cast<uint32_t>(iBound));
+        return;
+      }
+    }
+
     const size_t vertCapacity = buffer->ByteLength() / 4 / componentsPerVert;
     minVertCapacity = std::min(minVertCapacity, vertCapacity);
   }
 
   ////
 
   const auto& gl = mContext->gl;
   gl->fBeginTransformFeedback(primMode);
--- a/dom/canvas/test/webgl-conf/generated-mochitest.ini
+++ b/dom/canvas/test/webgl-conf/generated-mochitest.ini
@@ -7376,18 +7376,16 @@ subsuite = webgl2-ext
 [generated/test_2_conformance2__textures__webgl_canvas__tex-3d-srgb8_alpha8-rgba-unsigned_byte.html]
 subsuite = webgl2-ext
 [generated/test_2_conformance2__transform_feedback__default_transform_feedback.html]
 subsuite = webgl2-core
 [generated/test_2_conformance2__transform_feedback__non-existent-varying.html]
 subsuite = webgl2-core
 [generated/test_2_conformance2__transform_feedback__same-buffer-two-binding-points.html]
 subsuite = webgl2-core
-fail-if = 1
-skip-if = (os == 'win')
 [generated/test_2_conformance2__transform_feedback__simultaneous_binding.html]
 subsuite = webgl2-core
 [generated/test_2_conformance2__transform_feedback__switching-objects.html]
 subsuite = webgl2-core
 [generated/test_2_conformance2__transform_feedback__too-small-buffers.html]
 subsuite = webgl2-core
 skip-if = 1
 [generated/test_2_conformance2__transform_feedback__transform_feedback.html]
--- a/dom/canvas/test/webgl-conf/mochitest-errata.ini
+++ b/dom/canvas/test/webgl-conf/mochitest-errata.ini
@@ -171,20 +171,16 @@ fail-if = 1
 fail-if = 1
 [generated/test_2_conformance2__textures__video__tex-2d-rgb10_a2-rgba-unsigned_int_2_10_10_10_rev.html]
 fail-if = 1
 [generated/test_2_conformance2__textures__video__tex-3d-rgb10_a2-rgba-unsigned_int_2_10_10_10_rev.html]
 fail-if = 1
 
 [generated/test_2_conformance2__textures__misc__tex-unpack-params-with-flip-y-and-premultiply-alpha.html]
 fail-if = 1
-[generated/test_2_conformance2__transform_feedback__same-buffer-two-binding-points.html]
-fail-if = 1
-# ABORT_ON_ERROR from ANGLE
-skip-if = (os == 'win')
 
 ########################################################################
 # Complicated
 
 [generated/test_conformance__context__context-attributes-alpha-depth-stencil-antialias.html]
 # Asserts on linux debug. Crashes on Android.
 skip-if = (os == 'linux') || (os == 'android')