Bug 1425488 - Warn when ill-advised readbacks will cause pipeline stalls. - r=daoshengmu draft
authorJeff Gilbert <jgilbert@mozilla.com>
Fri, 15 Dec 2017 09:47:29 -0800
changeset 712082 d44e177195330062155e6df2187b91df9764f918
parent 712012 6d82e132348fbe33cf3eb7c85c485083c50c6bb9
child 743975 638235575235d38df971932d562ef0d5c08b666a
push id93250
push userbmo:jgilbert@mozilla.com
push dateFri, 15 Dec 2017 17:48:40 +0000
reviewersdaoshengmu
bugs1425488
milestone59.0a1
Bug 1425488 - Warn when ill-advised readbacks will cause pipeline stalls. - r=daoshengmu MozReview-Commit-ID: KXZIxzqNTc2
dom/canvas/WebGL2ContextBuffers.cpp
dom/canvas/WebGL2ContextSync.cpp
dom/canvas/WebGLBuffer.h
dom/canvas/WebGLContext.h
dom/canvas/WebGLContextGL.cpp
dom/canvas/WebGLSync.cpp
dom/canvas/WebGLSync.h
--- a/dom/canvas/WebGL2ContextBuffers.cpp
+++ b/dom/canvas/WebGL2ContextBuffers.cpp
@@ -123,16 +123,36 @@ WebGL2Context::GetBufferSubData(GLenum t
     if (!CheckedInt<GLsizeiptr>(byteLen).isValid()) {
         ErrorOutOfMemory("%s: Size too large.", funcName);
         return;
     }
     const GLsizeiptr glByteLen(byteLen);
 
     ////
 
+    switch (buffer->mUsage) {
+    case LOCAL_GL_STATIC_READ:
+    case LOCAL_GL_STREAM_READ:
+    case LOCAL_GL_DYNAMIC_READ:
+        if (mCompletedFenceId < buffer->mLastUpdateFenceId) {
+            GeneratePerfWarning("%s: Reading from a buffer without checking for previous"
+                                " command completion likely causes pipeline stalls."
+                                " Please use FenceSync.",
+                                funcName);
+        }
+        break;
+    default:
+        GeneratePerfWarning("%s: Reading from a buffer with usage other than *_READ"
+                            " causes pipeline stalls. Copy through a STREAM_READ buffer.",
+                            funcName);
+        break;
+    }
+
+    ////
+
     gl->MakeCurrent();
     const ScopedLazyBind readBind(gl, target, buffer);
 
     if (byteLen) {
         const bool isTF = (target == LOCAL_GL_TRANSFORM_FEEDBACK_BUFFER);
         GLenum mapTarget = target;
         if (isTF) {
             gl->fBindTransformFeedback(LOCAL_GL_TRANSFORM_FEEDBACK, mEmptyTFO);
--- a/dom/canvas/WebGL2ContextSync.cpp
+++ b/dom/canvas/WebGL2ContextSync.cpp
@@ -69,17 +69,25 @@ WebGL2Context::ClientWaitSync(const WebG
 
     if (timeout > kMaxClientWaitSyncTimeoutNS) {
         ErrorInvalidOperation("%s: `timeout` must not exceed %s nanoseconds.", funcName,
                               "MAX_CLIENT_WAIT_TIMEOUT_WEBGL");
         return LOCAL_GL_WAIT_FAILED;
     }
 
     MakeContextCurrent();
-    return gl->fClientWaitSync(sync.mGLName, flags, timeout);
+    const auto ret = gl->fClientWaitSync(sync.mGLName, flags, timeout);
+
+    if (ret == LOCAL_GL_CONDITION_SATISFIED ||
+        ret == LOCAL_GL_ALREADY_SIGNALED)
+    {
+        sync.MarkSignaled();
+    }
+
+    return ret;
 }
 
 void
 WebGL2Context::WaitSync(const WebGLSync& sync, GLbitfield flags, GLint64 timeout)
 {
     const char funcName[] = "waitSync";
     if (IsContextLost())
         return;
@@ -119,16 +127,23 @@ WebGL2Context::GetSyncParameter(JSContex
 
     GLint result = 0;
     switch (pname) {
     case LOCAL_GL_OBJECT_TYPE:
     case LOCAL_GL_SYNC_STATUS:
     case LOCAL_GL_SYNC_CONDITION:
     case LOCAL_GL_SYNC_FLAGS:
         gl->fGetSynciv(sync.mGLName, pname, 1, nullptr, &result);
+
+        if (pname == LOCAL_GL_SYNC_STATUS &&
+            result == LOCAL_GL_SIGNALED)
+        {
+            sync.MarkSignaled();
+        }
+
         retval.set(JS::Int32Value(result));
         return;
 
     default:
         ErrorInvalidEnum("%s: Invalid pname 0x%04x", funcName, pname);
         return;
     }
 }
--- a/dom/canvas/WebGLBuffer.h
+++ b/dom/canvas/WebGLBuffer.h
@@ -100,16 +100,17 @@ protected:
 
     void InvalidateCacheRange(uint64_t byteOffset, uint64_t byteLength) const;
 
     Kind mContent;
     GLenum mUsage;
     size_t mByteLength;
     size_t mTFBindCount;
     size_t mNonTFBindCount;
+    uint64_t mLastUpdateFenceId = 0;
 
     struct IndexRange final {
         GLenum type;
         uint64_t byteOffset;
         uint32_t indexCount;
 
         bool operator<(const IndexRange& x) const {
             if (type != x.type)
--- a/dom/canvas/WebGLContext.h
+++ b/dom/canvas/WebGLContext.h
@@ -309,16 +309,19 @@ class WebGLContext
         UNMASKED_VENDOR_WEBGL = 0x9245,
         UNMASKED_RENDERER_WEBGL = 0x9246
     };
 
     const uint32_t mMaxPerfWarnings;
     mutable uint64_t mNumPerfWarnings;
     const uint32_t mMaxAcceptableFBStatusInvals;
 
+    uint64_t mNextFenceId = 1;
+    uint64_t mCompletedFenceId = 0;
+
 public:
     WebGLContext();
 
 protected:
     virtual ~WebGLContext();
 
 public:
     NS_DECL_CYCLE_COLLECTING_ISUPPORTS
--- a/dom/canvas/WebGLContextGL.cpp
+++ b/dom/canvas/WebGLContextGL.cpp
@@ -2427,16 +2427,19 @@ WebGLContext::Flush() {
 }
 
 void
 WebGLContext::Finish() {
     if (IsContextLost())
         return;
     MakeContextCurrent();
     gl->fFinish();
+
+    mCompletedFenceId = mNextFenceId;
+    mNextFenceId += 1;
 }
 
 void
 WebGLContext::LineWidth(GLfloat width)
 {
     if (IsContextLost())
         return;
 
--- a/dom/canvas/WebGLSync.cpp
+++ b/dom/canvas/WebGLSync.cpp
@@ -8,41 +8,50 @@
 #include "GLContext.h"
 #include "mozilla/dom/WebGL2RenderingContextBinding.h"
 #include "WebGLContext.h"
 
 namespace mozilla {
 
 WebGLSync::WebGLSync(WebGLContext* webgl, GLenum condition, GLbitfield flags)
     : WebGLRefCountedObject(webgl)
+    , mGLName(mContext->gl->fFenceSync(condition, flags))
+    , mFenceId(mContext->mNextFenceId)
 {
-   mContext->mSyncs.insertBack(this);
-   mGLName = mContext->gl->fFenceSync(condition, flags);
+    mContext->mNextFenceId += 1;
+    mContext->mSyncs.insertBack(this);
 }
 
 WebGLSync::~WebGLSync()
 {
     DeleteOnce();
 }
 
 void
 WebGLSync::Delete()
 {
     mContext->MakeContextCurrent();
     mContext->gl->fDeleteSync(mGLName);
-    mGLName = 0;
     LinkedListElement<WebGLSync>::removeFrom(mContext->mSyncs);
 }
 
 WebGLContext*
 WebGLSync::GetParentObject() const
 {
     return mContext;
 }
 
+void
+WebGLSync::MarkSignaled() const
+{
+    if (mContext->mCompletedFenceId < mFenceId) {
+        mContext->mCompletedFenceId = mFenceId;
+    }
+}
+
 // -------------------------------------------------------------------------
 // IMPLEMENT NS
 JSObject*
 WebGLSync::WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto)
 {
     return dom::WebGLSyncBinding::Wrap(cx, this, givenProto);
 }
 
--- a/dom/canvas/WebGLSync.h
+++ b/dom/canvas/WebGLSync.h
@@ -14,28 +14,31 @@ namespace mozilla {
 
 class WebGLSync final
     : public nsWrapperCache
     , public WebGLRefCountedObject<WebGLSync>
     , public LinkedListElement<WebGLSync>
 {
     friend class WebGL2Context;
 
+    const GLsync mGLName;
+    const uint64_t mFenceId;
+
 public:
     WebGLSync(WebGLContext* webgl, GLenum condition, GLbitfield flags);
 
     void Delete();
     WebGLContext* GetParentObject() const;
 
     virtual JSObject* WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto) override;
 
     NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(WebGLSync)
     NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(WebGLSync)
 
+    void MarkSignaled() const;
+
 private:
     ~WebGLSync();
-
-    GLsync mGLName;
 };
 
 } // namespace mozilla
 
 #endif // WEBGL_SYNC_H_