Bug 1372157 - Fix IsFoo results in WebGL. r=kvark

Passes deqp/functional/gles3/lifetime.html.

Differential Revision: https://phabricator.services.mozilla.com/D8956

dom/canvas/WebGL2ContextSamplers.cpp

@@ -43,7 +43,10 @@ WebGL2Context::IsSampler(const WebGLSampler* const obj)
     if (!ValidateIsObject(obj))
         return false;
 
-    return gl->fIsSampler(obj->mGLName);
+    if (obj->IsDeleteRequested())
+        return false;
+
+    return true;
 }
 
 void

dom/canvas/WebGL2ContextTransformFeedback.cpp

@@ -54,7 +54,10 @@ WebGL2Context::IsTransformFeedback(const WebGLTransformFeedback* const obj)
     if (!ValidateIsObject(obj))
         return false;
 
-    return gl->fIsTransformFeedback(obj->mGLName);
+    if (obj->IsDeleteRequested())
+        return false;
+
+    return obj->mHasBeenBound;
 }
 
 void
@@ -90,6 +93,7 @@ WebGL2Context::BindTransformFeedback(GLenum target, WebGLTransformFeedback* tf)
 
     if (mBoundTransformFeedback) {
         mBoundTransformFeedback->AddBufferBindCounts(+1);
+        mBoundTransformFeedback->mHasBeenBound = true;
     }
 }
 

dom/canvas/WebGLContext.cpp

@@ -2493,7 +2493,10 @@ WebGLContext::ValidateIsObject(const WebGLDeletableObject* const object) const
     if (!object->IsCompatibleWithContext(this))
         return false;
 
-    return !object->IsDeleted();
+    if (object->IsDeleted())
+        return false;
+
+    return true;
 }
 
 bool

dom/canvas/WebGLContextGL.cpp

@@ -147,9 +147,7 @@ WebGLContext::BindFramebuffer(GLenum target, WebGLFramebuffer* wfb)
     } else {
         GLuint framebuffername = wfb->mGLName;
         gl->fBindFramebuffer(target, framebuffername);
-#ifdef ANDROID
-        wfb->mIsFB = true;
-#endif
+        wfb->mHasBeenBound = true;
     }
 
     switch (target) {
@@ -1065,7 +1063,10 @@ WebGLContext::IsBuffer(const WebGLBuffer* const obj)
     if (!ValidateIsObject(obj))
         return false;
 
-    return gl->fIsBuffer(obj->mGLName);
+    if (obj->IsDeleteRequested())
+        return false;
+
+    return obj->Content() != WebGLBuffer::Kind::Undefined;
 }
 
 bool
@@ -1075,15 +1076,10 @@ WebGLContext::IsFramebuffer(const WebGLFramebuffer* const obj)
     if (!ValidateIsObject(obj))
         return false;
 
-#ifdef ANDROID
-    if (gl->WorkAroundDriverBugs() &&
-        gl->Renderer() == GLRenderer::AndroidEmulator)
-    {
-        return obj->mIsFB;
-    }
-#endif
+    if (obj->IsDeleteRequested())
+        return false;
 
-    return gl->fIsFramebuffer(obj->mGLName);
+    return obj->mHasBeenBound;
 }
 
 bool
@@ -1100,7 +1096,10 @@ WebGLContext::IsQuery(const WebGLQuery* const obj)
     if (!ValidateIsObject(obj))
         return false;
 
-    return obj->IsQuery();
+    if (obj->IsDeleteRequested())
+        return false;
+
+    return bool(obj->Target());
 }
 
 bool
@@ -1110,6 +1109,9 @@ WebGLContext::IsRenderbuffer(const WebGLRenderbuffer* const obj)
     if (!ValidateIsObject(obj))
         return false;
 
+    if (obj->IsDeleteRequested())
+        return false;
+
     return obj->mHasBeenBound;
 }
 
@@ -1127,7 +1129,10 @@ WebGLContext::IsTexture(const WebGLTexture* const obj)
     if (!ValidateIsObject(obj))
         return false;
 
-    return obj->IsTexture();
+    if (obj->IsDeleteRequested())
+        return false;
+
+    return bool(obj->Target());
 }
 
 bool
@@ -1137,7 +1142,10 @@ WebGLContext::IsVertexArray(const WebGLVertexArray* const obj)
     if (!ValidateIsObject(obj))
         return false;
 
-    return obj->IsVertexArray();
+    if (obj->IsDeleteRequested())
+        return false;
+
+    return obj->mHasBeenBound;
 }
 
 // -

dom/canvas/WebGLContextUtils.cpp

@@ -702,8 +702,8 @@ WebGLContext::AssertCachedBindings() const
     GetAndFlushUnderlyingGLErrors();
 
     if (IsWebGL2() || IsExtensionEnabled(WebGLExtensionID::OES_vertex_array_object)) {
-        GLuint bound = mBoundVertexArray ? mBoundVertexArray->GLName() : 0;
-        AssertUintParamCorrect(gl, LOCAL_GL_VERTEX_ARRAY_BINDING, bound);
+        AssertUintParamCorrect(gl, LOCAL_GL_VERTEX_ARRAY_BINDING,
+                               mBoundVertexArray->mGLName);
     }
 
     GLint stencilBits = 0;

dom/canvas/WebGLContextValidate.cpp

@@ -626,10 +626,6 @@ WebGLContext::InitAndValidateGL(FailureReason* const out_failReason)
         return false;
     }
 
-    mDefaultVertexArray = WebGLVertexArray::Create(this);
-    mDefaultVertexArray->mAttribs.SetLength(mGLMaxVertexAttribs);
-    mBoundVertexArray = mDefaultVertexArray;
-
     // OpenGL core profiles remove the default VAO object from version
     // 4.0.0. We create a default VAO for all core profiles,
     // regardless of version.
@@ -638,11 +634,9 @@ WebGLContext::InitAndValidateGL(FailureReason* const out_failReason)
     // (https://www.opengl.org/registry/doc/glspec40.core.20100311.pdf)
     // in Section E.2.2 "Removed Features", pg 397: "[...] The default
     // vertex array object (the name zero) is also deprecated. [...]"
-
-    if (gl->IsCoreProfile()) {
-        mDefaultVertexArray->GenVertexArray();
-        mDefaultVertexArray->BindVertexArray();
-    }
+    mDefaultVertexArray = WebGLVertexArray::Create(this);
+    mDefaultVertexArray->BindVertexArray();
+    mDefaultVertexArray->mAttribs.SetLength(mGLMaxVertexAttribs);
 
     mPixelStore_FlipY = false;
     mPixelStore_PremultiplyAlpha = false;

dom/canvas/WebGLContextVertexArray.cpp

@@ -35,6 +35,7 @@ WebGLContext::BindVertexArray(WebGLVertexArray* array)
     MOZ_ASSERT(mBoundVertexArray == array);
     if (mBoundVertexArray) {
         mBoundVertexArray->AddBufferBindCounts(+1);
+        mBoundVertexArray->mHasBeenBound = true;
     }
 }
 
@@ -46,9 +47,6 @@ WebGLContext::CreateVertexArray()
         return nullptr;
 
     RefPtr<WebGLVertexArray> globj = CreateVertexArrayImpl();
-
-    globj->GenVertexArray();
-
     return globj.forget();
 }
 

dom/canvas/WebGLFramebuffer.cpp

@@ -508,10 +508,6 @@ WebGLFramebuffer::Delete()
     mContext->gl->fDeleteFramebuffers(1, &mGLName);
 
     LinkedListElement<WebGLFramebuffer>::removeFrom(mContext->mFramebuffers);
-
-#ifdef ANDROID
-    mIsFB = false;
-#endif
 }
 
 ////
@@ -1232,7 +1228,7 @@ WebGLFramebuffer::FramebufferTexture2D(GLenum attachEnum,
         if (!mContext->ValidateObject("texture", *tex))
             return;
 
-        if (!tex->HasEverBeenBound()) {
+        if (!tex->Target()) {
             mContext->ErrorInvalidOperation("`texture` has never been bound.");
             return;
         }
@@ -1317,7 +1313,7 @@ WebGLFramebuffer::FramebufferTextureLayer(GLenum attachEnum,
         if (!mContext->ValidateObject("texture", *tex))
             return;
 
-        if (!tex->HasEverBeenBound()) {
+        if (!tex->Target()) {
             mContext->ErrorInvalidOperation("`texture` has never been bound.");
             return;
         }

dom/canvas/WebGLFramebuffer.h

@@ -140,28 +140,18 @@ class WebGLFramebuffer final
     , public SupportsWeakPtr<WebGLFramebuffer>
     , public CacheInvalidator
 {
-    friend class WebGLContext;
-
 public:
     MOZ_DECLARE_WEAKREFERENCE_TYPENAME(WebGLFramebuffer)
 
     const GLuint mGLName;
+    bool mHasBeenBound = false;
 
 private:
     mutable uint64_t mNumFBStatusInvals = 0;
 
-protected:
-#ifdef ANDROID
-    // Bug 1140459: Some drivers (including our test slaves!) don't
-    // give reasonable answers for IsRenderbuffer, maybe others.
-    // This shows up on Android 2.3 emulator.
-    //
-    // So we track the `is a Framebuffer` state ourselves.
-    bool mIsFB = false;
-#endif
-
     ////
 
+protected:
     WebGLFBAttachPoint mDepthAttachment;
     WebGLFBAttachPoint mStencilAttachment;
     WebGLFBAttachPoint mDepthStencilAttachment;

dom/canvas/WebGLRenderbuffer.cpp

@@ -52,7 +52,6 @@ WebGLRenderbuffer::WebGLRenderbuffer(WebGLContext* webgl)
     , mPrimaryRB( DoCreateRenderbuffer(webgl->gl) )
     , mEmulatePackedDepthStencil( EmulatePackedDepthStencil(webgl->gl) )
     , mSecondaryRB(0)
-    , mHasBeenBound(false)
 {
     mContext->mRenderbuffers.insertBack(this);
 

dom/canvas/WebGLRenderbuffer.h

@@ -26,16 +26,15 @@ class WebGLRenderbuffer final
     , public WebGLRectangleObject
     , public CacheInvalidator
 {
-    friend class WebGLContext;
     friend class WebGLFramebuffer;
     friend class WebGLFBAttachPoint;
 
 public:
     const GLuint mPrimaryRB;
+    bool mHasBeenBound = false;
 protected:
     const bool mEmulatePackedDepthStencil;
     GLuint mSecondaryRB;
-    bool mHasBeenBound;
     webgl::ImageInfo mImageInfo;
 
 public:

dom/canvas/WebGLSampler.cpp

@@ -11,8 +11,6 @@
 
 namespace mozilla {
 
-
-
 WebGLSampler::WebGLSampler(WebGLContext* const webgl)
     : WebGLRefCountedObject(webgl)
     , mGLName([&]() {

dom/canvas/WebGLSampler.h

@@ -20,9 +20,6 @@ class WebGLSampler final
     , public LinkedListElement<WebGLSampler>
     , public CacheInvalidator
 {
-    friend class WebGLContext2;
-    friend class WebGLTexture;
-
     NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(WebGLSampler)
     NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(WebGLSampler)
 

dom/canvas/WebGLStrongTypes.h

@@ -155,6 +155,8 @@ public:
         return get() >= other.get();
     }
 
+    explicit operator bool() const { return bool(get()); }
+
     static bool IsValueLegal(GLenum value) {
         if (value > UINT16_MAX) {
             return false;

dom/canvas/WebGLTexture.cpp

@@ -698,7 +698,7 @@ WebGLTexture::BindTexture(TexTarget texTarget)
         return false;
     }
 
-    const bool isFirstBinding = !HasEverBeenBound();
+    const bool isFirstBinding = !mTarget;
     if (!isFirstBinding && mTarget != texTarget) {
         mContext->ErrorInvalidOperation("bindTexture: This texture has already been bound"
                                         " to a different target.");
@@ -856,12 +856,6 @@ WebGLTexture::GetTexParameter(TexTarget texTarget, GLenum pname)
     }
 }
 
-bool
-WebGLTexture::IsTexture() const
-{
-    return HasEverBeenBound() && !IsDeleted();
-}
-
 // Here we have to support all pnames with both int and float params.
 // See this discussion:
 //   https://www.khronos.org/webgl/public-mailing-list/archives/1008/msg00014.html

dom/canvas/WebGLTexture.h

@@ -172,7 +172,6 @@ public:
 
     void Delete();
 
-    bool HasEverBeenBound() const { return mTarget != LOCAL_GL_NONE; }
     TexTarget Target() const { return mTarget; }
 
     WebGLContext* GetParentObject() const {
@@ -192,7 +191,6 @@ public:
     bool BindTexture(TexTarget texTarget);
     void GenerateMipmap();
     JS::Value GetTexParameter(TexTarget texTarget, GLenum pname);
-    bool IsTexture() const;
     void TexParameter(TexTarget texTarget, GLenum pname, const FloatOrInt& param);
 
     ////////////////////////////////////

dom/canvas/WebGLTransformFeedback.h

@@ -30,6 +30,7 @@ class WebGLTransformFeedback final
 
 public:
     const GLuint mGLName;
+    bool mHasBeenBound = false;
 private:
     // GLES 3.0.4 p267, Table 6.24 "Transform Feedback State"
     // It's not yet in the ES3 spec, but the generic TF buffer bind point has been moved

dom/canvas/WebGLVertexArray.cpp

@@ -20,9 +20,9 @@ WebGLVertexArray::WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto)
     return dom::WebGLVertexArrayObject_Binding::Wrap(cx, this, givenProto);
 }
 
-WebGLVertexArray::WebGLVertexArray(WebGLContext* webgl)
+WebGLVertexArray::WebGLVertexArray(WebGLContext* const webgl, const GLuint name)
     : WebGLRefCountedObject(webgl)
-    , mGLName(0)
+    , mGLName(name)
 {
     mAttribs.SetLength(mContext->mGLMaxVertexAttribs);
     mContext->mVertexArrays.insertBack(this);
@@ -65,12 +65,6 @@ WebGLVertexArray::Delete()
     mAttribs.Clear();
 }
 
-bool
-WebGLVertexArray::IsVertexArray() const
-{
-    return IsVertexArrayImpl();
-}
-
 NS_IMPL_CYCLE_COLLECTION_WRAPPERCACHE(WebGLVertexArray,
                                       mAttribs,
                                       mElementArrayBuffer)

dom/canvas/WebGLVertexArray.h

@@ -31,15 +31,7 @@ class WebGLVertexArray
 public:
     static WebGLVertexArray* Create(WebGLContext* webgl);
 
-    void BindVertexArray() {
-        // Bind to dummy value to signal that this vertex array has ever been
-        // bound.
-        BindVertexArrayImpl();
-    };
-
-    // Implement parent classes:
     void Delete();
-    bool IsVertexArray() const;
 
     WebGLContext* GetParentObject() const {
         return mContext;
@@ -50,20 +42,19 @@ public:
     NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(WebGLVertexArray)
     NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(WebGLVertexArray)
 
-    GLuint GLName() const { return mGLName; }
-
     void AddBufferBindCounts(int8_t addVal) const;
 
 protected:
-    explicit WebGLVertexArray(WebGLContext* webgl);
+    WebGLVertexArray(WebGLContext* webgl, GLuint name);
     virtual ~WebGLVertexArray();
 
-    virtual void GenVertexArray() = 0;
-    virtual void BindVertexArrayImpl() = 0;
+    virtual void BindVertexArray() = 0;
     virtual void DeleteImpl() = 0;
-    virtual bool IsVertexArrayImpl() const = 0;
 
-    GLuint mGLName;
+public:
+    const GLuint mGLName;
+    bool mHasBeenBound = false;
+protected:
     nsTArray<WebGLVertexAttribData> mAttribs;
     WebGLRefPtr<WebGLBuffer> mElementArrayBuffer;
 

dom/canvas/WebGLVertexArrayFake.cpp

@@ -11,12 +11,11 @@
 namespace mozilla {
 
 WebGLVertexArrayFake::WebGLVertexArrayFake(WebGLContext* webgl)
-    : WebGLVertexArray(webgl)
-    , mIsVAO(false)
+    : WebGLVertexArray(webgl, 0)
 { }
 
 void
-WebGLVertexArrayFake::BindVertexArrayImpl()
+WebGLVertexArrayFake::BindVertexArray()
 {
     // Go through and re-bind all buffers and setup all
     // vertex attribute pointers
@@ -52,19 +51,6 @@ WebGLVertexArrayFake::BindVertexArrayImpl()
     }
 
     mContext->BindBuffer(LOCAL_GL_ARRAY_BUFFER, prevBuffer);
-    mIsVAO = true;
-}
-
-void
-WebGLVertexArrayFake::DeleteImpl()
-{
-    mIsVAO = false;
-}
-
-bool
-WebGLVertexArrayFake::IsVertexArrayImpl() const
-{
-    return mIsVAO;
 }
 
 } // namespace mozilla

dom/canvas/WebGLVertexArrayFake.h

@@ -16,10 +16,8 @@ class WebGLVertexArrayFake final
     friend class WebGLVertexArray;
 
 protected:
-    virtual void BindVertexArrayImpl() override;
-    virtual void DeleteImpl() override;
-    virtual void GenVertexArray() override {};
-    virtual bool IsVertexArrayImpl() const override;
+    virtual void BindVertexArray() override;
+    virtual void DeleteImpl() override {}
 
 private:
     explicit WebGLVertexArrayFake(WebGLContext* webgl);
@@ -27,8 +25,6 @@ private:
     ~WebGLVertexArrayFake() {
         DeleteOnce();
     }
-
-    bool mIsVAO;
 };
 
 } // namespace mozilla

dom/canvas/WebGLVertexArrayGL.cpp

@@ -11,8 +11,12 @@
 namespace mozilla {
 
 WebGLVertexArrayGL::WebGLVertexArrayGL(WebGLContext* webgl)
-    : WebGLVertexArray(webgl)
-    , mIsVAO(false)
+    : WebGLVertexArray(webgl,
+                       [&]() {
+                           GLuint ret = 0;
+                           webgl->gl->fGenVertexArrays(1, &ret);
+                           return ret;
+                       }())
 { }
 
 WebGLVertexArrayGL::~WebGLVertexArrayGL()
@@ -26,35 +30,13 @@ WebGLVertexArrayGL::DeleteImpl()
     mElementArrayBuffer = nullptr;
 
     mContext->gl->fDeleteVertexArrays(1, &mGLName);
-
-    mIsVAO = false;
 }
 
 void
-WebGLVertexArrayGL::BindVertexArrayImpl()
+WebGLVertexArrayGL::BindVertexArray()
 {
     mContext->mBoundVertexArray = this;
     mContext->gl->fBindVertexArray(mGLName);
-
-    mIsVAO = true;
-}
-
-void
-WebGLVertexArrayGL::GenVertexArray()
-{
-    mContext->gl->fGenVertexArrays(1, &mGLName);
-}
-
-bool
-WebGLVertexArrayGL::IsVertexArrayImpl() const
-{
-    gl::GLContext* gl = mContext->gl;
-    if (gl->WorkAroundDriverBugs())
-    {
-        return mIsVAO;
-    }
-
-    return mContext->gl->fIsVertexArray(mGLName) != 0;
 }
 
 } // namespace mozilla

dom/canvas/WebGLVertexArrayGL.h

@@ -17,19 +17,11 @@ class WebGLVertexArrayGL
 
 public:
     virtual void DeleteImpl() override;
-    virtual void BindVertexArrayImpl() override;
-    virtual void GenVertexArray() override;
-    virtual bool IsVertexArrayImpl() const override;
+    virtual void BindVertexArray() override;
 
 protected:
     explicit WebGLVertexArrayGL(WebGLContext* webgl);
     ~WebGLVertexArrayGL();
-
-    // Bug 1140459: Some drivers (including our test slaves!) don't
-    // give reasonable answers for IsVertexArray, maybe others.
-    //
-    // So we track the `is a VAO` state ourselves.
-    bool mIsVAO;
 };
 
 } // namespace mozilla

dom/canvas/test/webgl-conf/checkout/conformance/extensions/oes-vertex-array-object.html

@@ -645,12 +645,13 @@ function runBoundDeleteTests() {
         wtu.glErrorShouldBe(gl, expectedError,
                             "Draw call should " + (expectRetained ? "not " : "") + "fail.");
 
-        if (!gl.isBuffer(positionBuffer)) {
-            testFailed("References from unbound VAOs keep Position buffer alive.");
-        }
-        if (!gl.isBuffer(colorBuffer)) {
-            testFailed("References from unbound VAOs keep Color buffer alive");
-        }
+        // https://github.com/KhronosGroup/WebGL/pull/2731
+        //if (!gl.isBuffer(positionBuffer)) {
+        //    testFailed("References from unbound VAOs keep Position buffer alive.");
+        //}
+        //if (!gl.isBuffer(colorBuffer)) {
+        //    testFailed("References from unbound VAOs keep Color buffer alive");
+        //}
     }
 }