Bug 1300946 - Forbid simultaneous binding to TF and non-TF bind points. - r=jrmuizel draft
authorJeff Gilbert (:jgilbert) <jgilbert@mozilla.com>
Sat, 17 Sep 2016 16:44:56 -0700
changeset 416665 d224f94b9723e1a9f5f4e7ada0f12c1f2a4855fc
parent 416664 45afa8079de8725bdb7c2099f5d9e283ff38c668
child 416666 8ae9b6fcb4ed7bda0c3fd06638575a54764c7a99
push id30211
push userbmo:jgilbert@mozilla.com
push dateThu, 22 Sep 2016 19:02:05 +0000
reviewersjrmuizel
bugs1300946
milestone51.0a1
Bug 1300946 - Forbid simultaneous binding to TF and non-TF bind points. - r=jrmuizel
dom/canvas/WebGLBuffer.cpp
dom/canvas/WebGLBuffer.h
dom/canvas/WebGLContextBuffers.cpp
--- a/dom/canvas/WebGLBuffer.cpp
+++ b/dom/canvas/WebGLBuffer.cpp
@@ -13,16 +13,17 @@
 namespace mozilla {
 
 WebGLBuffer::WebGLBuffer(WebGLContext* webgl, GLuint buf)
     : WebGLContextBoundObject(webgl)
     , mGLName(buf)
     , mContent(Kind::Undefined)
     , mByteLength(0)
     , mNumActiveTFOs(0)
+    , mBoundForTF(false)
 {
     mContext->mBuffers.insertBack(this);
 }
 
 WebGLBuffer::~WebGLBuffer()
 {
     MOZ_ASSERT(!mNumActiveTFOs);
     DeleteOnce();
@@ -184,16 +185,78 @@ WebGLBuffer::Validate(GLenum type, uint3
 }
 
 bool
 WebGLBuffer::IsElementArrayUsedWithMultipleTypes() const
 {
     return mCache->BeenUsedWithMultipleTypes();
 }
 
+bool
+WebGLBuffer::ValidateCanBindToTarget(const char* funcName, GLenum target)
+{
+    const bool wouldBeTF = (target == LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER);
+    if (mWebGLRefCnt && wouldBeTF != mBoundForTF) {
+        mContext->ErrorInvalidOperation("%s: Buffers cannot be simultaneously bound to "
+                                        " transform feedback and bound elsewhere.",
+                                        funcName);
+        return false;
+    }
+    mBoundForTF = wouldBeTF;
+
+    /* 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
+     * binding point except COPY_READ_BUFFER or COPY_WRITE_BUFFER will
+     * then set the WebGL buffer type of the buffer being bound
+     * according to the table above.
+     *
+     * Any call to one of these functions which attempts to bind a
+     * WebGLBuffer that has the element array WebGL buffer type to a
+     * binding point that falls under other data, or bind a
+     * WebGLBuffer which has the other data WebGL buffer type to
+     * ELEMENT_ARRAY_BUFFER will generate an INVALID_OPERATION error,
+     * and the state of the binding point will remain untouched.
+     */
+
+    if (mContent == WebGLBuffer::Kind::Undefined)
+        return true;
+
+    switch (target) {
+    case LOCAL_GL_COPY_READ_BUFFER:
+    case LOCAL_GL_COPY_WRITE_BUFFER:
+        return true;
+
+    case LOCAL_GL_ELEMENT_ARRAY_BUFFER:
+        if (mContent == WebGLBuffer::Kind::ElementArray)
+            return true;
+        break;
+
+    case LOCAL_GL_ARRAY_BUFFER:
+    case LOCAL_GL_PIXEL_PACK_BUFFER:
+    case LOCAL_GL_PIXEL_UNPACK_BUFFER:
+    case LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER:
+    case LOCAL_GL_UNIFORM_BUFFER:
+        if (mContent == WebGLBuffer::Kind::OtherData)
+            return true;
+        break;
+
+    default:
+        MOZ_CRASH();
+    }
+
+    const auto dataType = (mContent == WebGLBuffer::Kind::OtherData) ? "other"
+                                                                     : "element";
+    mContext->ErrorInvalidOperation("%s: Buffer already contains %s data.", funcName,
+                                    dataType);
+    return false;
+}
+
 JSObject*
 WebGLBuffer::WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto)
 {
     return dom::WebGLBufferBinding::Wrap(cx, this, givenProto);
 }
 
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE_0(WebGLBuffer)
 
--- a/dom/canvas/WebGLBuffer.h
+++ b/dom/canvas/WebGLBuffer.h
@@ -57,27 +57,29 @@ public:
     bool IsElementArrayUsedWithMultipleTypes() const;
 
     WebGLContext* GetParentObject() const {
         return mContext;
     }
 
     virtual JSObject* WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto) override;
 
+    bool ValidateCanBindToTarget(const char* funcName, GLenum target);
     void BufferData(GLenum target, size_t size, const void* data, GLenum usage);
 
     const GLenum mGLName;
 
     NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(WebGLBuffer)
     NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(WebGLBuffer)
 
 protected:
     ~WebGLBuffer();
 
     Kind mContent;
     size_t mByteLength;
     UniquePtr<WebGLElementArrayCache> mCache;
     size_t mNumActiveTFOs;
+    bool mBoundForTF;
 };
 
 } // namespace mozilla
 
 #endif // WEBGL_BUFFER_H_
--- a/dom/canvas/WebGLContextBuffers.cpp
+++ b/dom/canvas/WebGLContextBuffers.cpp
@@ -104,73 +104,16 @@ WebGLContext::ValidateIndexedBufferSlot(
         return nullptr;
     }
 
     return &(*bindings)[index];
 }
 
 ////////////////////////////////////////
 
-static bool
-ValidateCanBindToTarget(WebGLContext* webgl, const char* funcName, GLenum target,
-                        WebGLBuffer* buffer)
-{
-    if (!buffer)
-        return true;
-
-    /* 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
-     * binding point except COPY_READ_BUFFER or COPY_WRITE_BUFFER will
-     * then set the WebGL buffer type of the buffer being bound
-     * according to the table above.
-     *
-     * Any call to one of these functions which attempts to bind a
-     * WebGLBuffer that has the element array WebGL buffer type to a
-     * binding point that falls under other data, or bind a
-     * WebGLBuffer which has the other data WebGL buffer type to
-     * ELEMENT_ARRAY_BUFFER will generate an INVALID_OPERATION error,
-     * and the state of the binding point will remain untouched.
-     */
-
-    const auto& content = buffer->Content();
-    if (content == WebGLBuffer::Kind::Undefined)
-        return true;
-
-    switch (target) {
-    case LOCAL_GL_COPY_READ_BUFFER:
-    case LOCAL_GL_COPY_WRITE_BUFFER:
-        return true;
-
-    case LOCAL_GL_ELEMENT_ARRAY_BUFFER:
-        if (content == WebGLBuffer::Kind::ElementArray)
-            return true;
-        break;
-
-    case LOCAL_GL_ARRAY_BUFFER:
-    case LOCAL_GL_PIXEL_PACK_BUFFER:
-    case LOCAL_GL_PIXEL_UNPACK_BUFFER:
-    case LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER:
-    case LOCAL_GL_UNIFORM_BUFFER:
-        if (content == WebGLBuffer::Kind::OtherData)
-            return true;
-        break;
-
-    default:
-        MOZ_CRASH();
-    }
-
-    webgl->ErrorInvalidOperation("%s: buffer already contains %s data.", funcName,
-                                 content == WebGLBuffer::Kind::OtherData ? "other"
-                                                                         : "element");
-    return false;
-}
-
 void
 WebGLContext::BindBuffer(GLenum target, WebGLBuffer* buffer)
 {
     const char funcName[] = "bindBuffer";
     if (IsContextLost())
         return;
 
     if (!ValidateObjectAllowDeletedOrNull(funcName, buffer))
@@ -179,17 +122,17 @@ WebGLContext::BindBuffer(GLenum target, 
     // silently ignore a deleted buffer
     if (buffer && buffer->IsDeleted())
         return;
 
     const auto& slot = ValidateBufferSlot(funcName, target);
     if (!slot)
         return;
 
-    if (!ValidateCanBindToTarget(this, funcName, target, buffer))
+    if (buffer && !buffer->ValidateCanBindToTarget(funcName, target))
         return;
 
     gl->MakeCurrent();
     gl->fBindBuffer(target, buffer ? buffer->mGLName : 0);
 
     *slot = buffer;
     if (buffer) {
         buffer->SetContentAfterBind(target);
@@ -216,19 +159,16 @@ WebGLContext::ValidateIndexedBufferBindi
         mBoundTransformFeedback->mIsActive)
     {
         ErrorInvalidOperation("%s: Cannot update indexed buffer bindings on active"
                               " transform feedback objects.",
                               funcName);
         return false;
     }
 
-    if (!ValidateCanBindToTarget(this, funcName, target, (*out_genericBinding)->get()))
-        return false;
-
     return true;
 }
 
 void
 WebGLContext::BindBufferBase(GLenum target, GLuint index, WebGLBuffer* buffer)
 {
     const char funcName[] = "bindBufferBase";
     if (IsContextLost())
@@ -244,16 +184,19 @@ WebGLContext::BindBufferBase(GLenum targ
     WebGLRefPtr<WebGLBuffer>* genericBinding;
     IndexedBufferBinding* indexedBinding;
     if (!ValidateIndexedBufferBinding(funcName, target, index, &genericBinding,
                                       &indexedBinding))
     {
         return;
     }
 
+    if (buffer && !buffer->ValidateCanBindToTarget(funcName, target))
+        return;
+
     ////
 
     gl->MakeCurrent();
     gl->fBindBufferBase(target, index, buffer ? buffer->mGLName : 0);
 
     ////
 
     *genericBinding = buffer;
@@ -290,16 +233,21 @@ WebGLContext::BindBufferRange(GLenum tar
     WebGLRefPtr<WebGLBuffer>* genericBinding;
     IndexedBufferBinding* indexedBinding;
     if (!ValidateIndexedBufferBinding(funcName, target, index, &genericBinding,
                                       &indexedBinding))
     {
         return;
     }
 
+    if (buffer && !buffer->ValidateCanBindToTarget(funcName, target))
+        return;
+
+    ////
+
     gl->MakeCurrent();
 
     switch (target) {
     case LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER:
         if (offset % 4 != 0 || size % 4 != 0) {
             ErrorInvalidValue("%s: For %s, `offset` and `size` must be multiples of 4.",
                               funcName, "TRANSFORM_FEEDBACK_BUFFER");
             return;