Bug 1303878 - Fix todos in WebGL. - r=mtseng a=ritu
☠☠ backed out by ca2593daaa91 ☠ ☠
authorJeff Gilbert <jgilbert@mozilla.com>
Mon, 18 Jul 2016 00:15:54 -0700
changeset 350508 379b19526d6a16356ab24c41d4d473225887b0f2
parent 350507 f4b45ea3c0c836e5a457c8009bff423447e54803
child 350509 a9dc19ea4d04e04c64b1c53da7a073102d276b80
push id1230
push userjlund@mozilla.com
push dateMon, 31 Oct 2016 18:13:35 +0000
treeherdermozilla-release@5e06e3766db2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmtseng, ritu
bugs1303878
milestone50.0
Bug 1303878 - Fix todos in WebGL. - r=mtseng a=ritu MozReview-Commit-ID: D15kLfHERAI
dom/canvas/WebGL1Context.cpp
dom/canvas/WebGL2Context.h
dom/canvas/WebGLContext.cpp
dom/canvas/WebGLContextGL.cpp
dom/canvas/WebGLFramebuffer.cpp
dom/canvas/WebGLFramebuffer.h
dom/canvas/WebGLRenderbuffer.cpp
dom/canvas/WebGLRenderbuffer.h
--- a/dom/canvas/WebGL1Context.cpp
+++ b/dom/canvas/WebGL1Context.cpp
@@ -36,17 +36,16 @@ JSObject*
 WebGL1Context::WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto)
 {
     return dom::WebGLRenderingContextBinding::Wrap(cx, this, givenProto);
 }
 
 bool
 WebGL1Context::ValidateQueryTarget(GLenum target, const char* info)
 {
-    // TODO: Implement this for EXT_disjoint_timer
     return false;
 }
 
 } // namespace mozilla
 
 nsresult
 NS_NewCanvasRenderingContextWebGL(nsIDOMWebGLRenderingContext** out_result)
 {
--- a/dom/canvas/WebGL2Context.h
+++ b/dom/canvas/WebGL2Context.h
@@ -268,31 +268,30 @@ public:
     void VertexAttribI4i(GLuint index, GLint x, GLint y, GLint z, GLint w);
     void VertexAttribI4iv(GLuint index, const dom::Sequence<GLint>& v);
     void VertexAttribI4ui(GLuint index, GLuint x, GLuint y, GLuint z, GLuint w);
     void VertexAttribI4uiv(GLuint index, const dom::Sequence<GLuint>& v);
 
 
     // -------------------------------------------------------------------------
     // Writing to the drawing buffer
-    // TODO(djg): Implemented in WebGLContext
-/*
+
+    /* Implemented in WebGLContext
     void VertexAttribDivisor(GLuint index, GLuint divisor);
     void DrawArraysInstanced(GLenum mode, GLint first, GLsizei count, GLsizei instanceCount);
     void DrawElementsInstanced(GLenum mode, GLsizei count, GLenum type, GLintptr offset, GLsizei instanceCount);
-*/
+    */
     void DrawRangeElements(GLenum mode, GLuint start, GLuint end, GLsizei count, GLenum type, GLintptr offset);
 
 
     // ------------------------------------------------------------------------
     // Multiple Render Targets - WebGL2ContextMRTs.cpp
-    // TODO(djg): Implemented in WebGLContext
-/*
+    /* Implemented in WebGLContext
     void DrawBuffers(const dom::Sequence<GLenum>& buffers);
-*/
+    */
 
     void ClearBufferiv_base(GLenum buffer, GLint drawbuffer, const GLint* value);
     void ClearBufferuiv_base(GLenum buffer, GLint drawbuffer, const GLuint* value);
     void ClearBufferfv_base(GLenum buffer, GLint drawbuffer, const GLfloat* value);
 
     void ClearBufferiv(GLenum buffer, GLint drawbuffer, const dom::Int32Array& value);
     void ClearBufferiv(GLenum buffer, GLint drawbuffer, const dom::Sequence<GLint>& value);
     void ClearBufferuiv(GLenum buffer, GLint drawbuffer, const dom::Uint32Array& value);
--- a/dom/canvas/WebGLContext.cpp
+++ b/dom/canvas/WebGLContext.cpp
@@ -1607,18 +1607,21 @@ WebGLContext::PresentScreenBuffer()
 }
 
 void
 WebGLContext::DummyReadFramebufferOperation(const char* funcName)
 {
     if (!mBoundReadFramebuffer)
         return; // Infallible.
 
+    const auto target = (IsWebGL2() ? LOCAL_GL_READ_FRAMEBUFFER
+                                    : LOCAL_GL_FRAMEBUFFER);
     nsCString fbStatusInfo;
-    const auto status = mBoundReadFramebuffer->CheckFramebufferStatus(&fbStatusInfo);
+    const auto status = mBoundReadFramebuffer->CheckFramebufferStatus(target,
+                                                                      &fbStatusInfo);
 
     if (status != LOCAL_GL_FRAMEBUFFER_COMPLETE) {
         nsCString errorText("Incomplete framebuffer");
 
         if (fbStatusInfo.Length()) {
             errorText += ": ";
             errorText += fbStatusInfo;
         }
@@ -1914,18 +1917,28 @@ WebGLContext::GetSurfaceSnapshot(bool* o
     if (NS_WARN_IF(!surf)) {
         return nullptr;
     }
 
     gl->MakeCurrent();
     {
         ScopedBindFramebuffer autoFB(gl, 0);
         ClearBackbufferIfNeeded();
-        // TODO: Save, override, then restore glReadBuffer if present.
+
+        // Save, override, then restore glReadBuffer.
+        const GLenum readBufferMode = gl->Screen()->GetReadBufferMode();
+
+        if (readBufferMode != LOCAL_GL_BACK) {
+            gl->fReadBuffer(LOCAL_GL_BACK);
+        }
         ReadPixelsIntoDataSurface(gl, surf);
+
+        if (readBufferMode != LOCAL_GL_BACK) {
+            gl->fReadBuffer(readBufferMode);
+        }
     }
 
     if (out_premultAlpha) {
         *out_premultAlpha = true;
     }
     bool srcPremultAlpha = mOptions.premultipliedAlpha;
     if (!srcPremultAlpha) {
         if (out_premultAlpha) {
--- a/dom/canvas/WebGLContextGL.cpp
+++ b/dom/canvas/WebGLContextGL.cpp
@@ -272,17 +272,17 @@ WebGLContext::CheckFramebufferStatus(GLe
     default:
         MOZ_CRASH("GFX: Bad target.");
     }
 
     if (!fb)
         return LOCAL_GL_FRAMEBUFFER_COMPLETE;
 
     nsCString fbErrorInfo;
-    return fb->CheckFramebufferStatus(&fbErrorInfo).get();
+    return fb->CheckFramebufferStatus(target, &fbErrorInfo).get();
 }
 
 already_AddRefed<WebGLProgram>
 WebGLContext::CreateProgram()
 {
     if (IsContextLost())
         return nullptr;
     RefPtr<WebGLProgram> globj = new WebGLProgram(this);
--- a/dom/canvas/WebGLFramebuffer.cpp
+++ b/dom/canvas/WebGLFramebuffer.cpp
@@ -338,27 +338,28 @@ WebGLFBAttachPoint::IsComplete(WebGLCont
             return false;
         }
     }
 
     return true;
 }
 
 void
-WebGLFBAttachPoint::FinalizeAttachment(gl::GLContext* gl, GLenum attachment) const
+WebGLFBAttachPoint::FinalizeAttachment(gl::GLContext* gl, FBTarget target,
+                                       GLenum attachment) const
 {
     if (!HasImage()) {
         switch (attachment) {
         case LOCAL_GL_DEPTH_ATTACHMENT:
         case LOCAL_GL_STENCIL_ATTACHMENT:
         case LOCAL_GL_DEPTH_STENCIL_ATTACHMENT:
             break;
 
         default:
-            gl->fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, attachment,
+            gl->fFramebufferRenderbuffer(target, attachment,
                                          LOCAL_GL_RENDERBUFFER, 0);
             break;
         }
 
         return;
     }
     MOZ_ASSERT(HasImage());
 
@@ -374,47 +375,47 @@ WebGLFBAttachPoint::FinalizeAttachment(g
         case LOCAL_GL_TEXTURE_2D:
         case LOCAL_GL_TEXTURE_CUBE_MAP_POSITIVE_X:
         case LOCAL_GL_TEXTURE_CUBE_MAP_NEGATIVE_X:
         case LOCAL_GL_TEXTURE_CUBE_MAP_POSITIVE_Y:
         case LOCAL_GL_TEXTURE_CUBE_MAP_NEGATIVE_Y:
         case LOCAL_GL_TEXTURE_CUBE_MAP_POSITIVE_Z:
         case LOCAL_GL_TEXTURE_CUBE_MAP_NEGATIVE_Z:
             if (attachment == LOCAL_GL_DEPTH_STENCIL_ATTACHMENT) {
-                gl->fFramebufferTexture2D(LOCAL_GL_FRAMEBUFFER, LOCAL_GL_DEPTH_ATTACHMENT,
+                gl->fFramebufferTexture2D(target, LOCAL_GL_DEPTH_ATTACHMENT,
                                           imageTarget, glName, mipLevel);
-                gl->fFramebufferTexture2D(LOCAL_GL_FRAMEBUFFER,
+                gl->fFramebufferTexture2D(target,
                                           LOCAL_GL_STENCIL_ATTACHMENT, imageTarget,
                                           glName, mipLevel);
             } else {
-                gl->fFramebufferTexture2D(LOCAL_GL_FRAMEBUFFER, attachment, imageTarget,
+                gl->fFramebufferTexture2D(target, attachment, imageTarget,
                                           glName, mipLevel);
             }
             break;
 
         case LOCAL_GL_TEXTURE_2D_ARRAY:
         case LOCAL_GL_TEXTURE_3D:
             if (attachment == LOCAL_GL_DEPTH_STENCIL_ATTACHMENT) {
-                gl->fFramebufferTextureLayer(LOCAL_GL_FRAMEBUFFER,
+                gl->fFramebufferTextureLayer(target,
                                              LOCAL_GL_DEPTH_ATTACHMENT, glName, mipLevel,
                                              layer);
-                gl->fFramebufferTextureLayer(LOCAL_GL_FRAMEBUFFER,
+                gl->fFramebufferTextureLayer(target,
                                              LOCAL_GL_STENCIL_ATTACHMENT, glName,
                                              mipLevel, layer);
             } else {
-                gl->fFramebufferTextureLayer(LOCAL_GL_FRAMEBUFFER, attachment, glName,
+                gl->fFramebufferTextureLayer(target, attachment, glName,
                                              mipLevel, layer);
             }
             break;
         }
         return;
     }
 
     if (Renderbuffer()) {
-        Renderbuffer()->DoFramebufferRenderbuffer(attachment);
+        Renderbuffer()->DoFramebufferRenderbuffer(target, attachment);
         return;
     }
 
     MOZ_CRASH("GFX: invalid render buffer");
 }
 
 JS::Value
 WebGLFBAttachPoint::GetParameter(const char* funcName, WebGLContext* webgl, JSContext* cx,
@@ -1002,33 +1003,32 @@ WebGLFramebuffer::PrecheckFramebufferSta
         if (depthOrStencilCount > 1)
             return LOCAL_GL_FRAMEBUFFER_UNSUPPORTED;
     }
 
     return LOCAL_GL_FRAMEBUFFER_COMPLETE;
 }
 
 FBStatus
-WebGLFramebuffer::CheckFramebufferStatus(nsCString* const out_info) const
+WebGLFramebuffer::CheckFramebufferStatus(FBTarget target, nsCString* const out_info) const
 {
     if (mIsKnownFBComplete)
         return LOCAL_GL_FRAMEBUFFER_COMPLETE;
 
     FBStatus ret = PrecheckFramebufferStatus(out_info);
     if (ret != LOCAL_GL_FRAMEBUFFER_COMPLETE)
         return ret;
 
     // Looks good on our end. Let's ask the driver.
     mContext->MakeContextCurrent();
 
     // Ok, attach our chosen flavor of {DEPTH, STENCIL, DEPTH_STENCIL}.
     FinalizeAttachments();
 
-    // TODO: This should not be unconditionally GL_FRAMEBUFFER.
-    ret = mContext->gl->fCheckFramebufferStatus(LOCAL_GL_FRAMEBUFFER);
+    ret = mContext->gl->fCheckFramebufferStatus(target);
 
     if (ret == LOCAL_GL_FRAMEBUFFER_COMPLETE) {
         mIsKnownFBComplete = true;
     } else {
         out_info->AssignLiteral("Bad status according to the driver");
     }
 
     return ret;
@@ -1207,40 +1207,48 @@ FinalizeDrawAndReadBuffers(gl::GLContext
     // TODO(djg): Assert that fDrawBuffer/fReadBuffer is not NULL.
     GLenum colorBufferSource = isColorBufferDefined ? LOCAL_GL_COLOR_ATTACHMENT0
                                                     : LOCAL_GL_NONE;
     gl->fDrawBuffer(colorBufferSource);
     gl->fReadBuffer(colorBufferSource);
 }
 
 void
-WebGLFramebuffer::FinalizeAttachments() const
+WebGLFramebuffer::FinalizeAttachments(FBTarget target) const
 {
-    MOZ_ASSERT(mContext->mBoundDrawFramebuffer == this ||
-               mContext->mBoundReadFramebuffer == this);
+    MOZ_ASSERT_IF(target == LOCAL_GL_READ_FRAMEBUFFER,
+                  mContext->mBoundReadFramebuffer == this);
+    MOZ_ASSERT_IF(target != LOCAL_GL_READ_FRAMEBUFFER,
+                  mContext->mBoundDrawFramebuffer == this);
 
     gl::GLContext* gl = mContext->gl;
 
-    // Nuke the depth and stencil attachment points.
-    gl->fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, LOCAL_GL_DEPTH_ATTACHMENT,
-                                 LOCAL_GL_RENDERBUFFER, 0);
-    gl->fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, LOCAL_GL_STENCIL_ATTACHMENT,
-                                 LOCAL_GL_RENDERBUFFER, 0);
+    ////
 
-    // Call finalize.
-    mColorAttachment0.FinalizeAttachment(gl, LOCAL_GL_COLOR_ATTACHMENT0);
-    mDepthAttachment.FinalizeAttachment(gl, LOCAL_GL_DEPTH_ATTACHMENT);
-    mStencilAttachment.FinalizeAttachment(gl, LOCAL_GL_STENCIL_ATTACHMENT);
-    mDepthStencilAttachment.FinalizeAttachment(gl, LOCAL_GL_DEPTH_STENCIL_ATTACHMENT);
+    mColorAttachment0.FinalizeAttachment(gl, target, LOCAL_GL_COLOR_ATTACHMENT0);
 
     for (size_t i = 0; i < mMoreColorAttachments.Size(); i++) {
         GLenum attachPoint = LOCAL_GL_COLOR_ATTACHMENT1 + i;
-        mMoreColorAttachments[i].FinalizeAttachment(gl, attachPoint);
+        mMoreColorAttachments[i].FinalizeAttachment(gl, target, attachPoint);
     }
 
+    ////
+
+    // Nuke the depth and stencil attachment points.
+    gl->fFramebufferRenderbuffer(target, LOCAL_GL_DEPTH_ATTACHMENT,
+                                 LOCAL_GL_RENDERBUFFER, 0);
+    gl->fFramebufferRenderbuffer(target, LOCAL_GL_STENCIL_ATTACHMENT,
+                                 LOCAL_GL_RENDERBUFFER, 0);
+
+    mDepthAttachment.FinalizeAttachment(gl, target, LOCAL_GL_DEPTH_ATTACHMENT);
+    mStencilAttachment.FinalizeAttachment(gl, target, LOCAL_GL_STENCIL_ATTACHMENT);
+    mDepthStencilAttachment.FinalizeAttachment(gl, target, LOCAL_GL_DEPTH_STENCIL_ATTACHMENT);
+
+    ////
+
     FinalizeDrawAndReadBuffers(gl, mColorAttachment0.IsDefined());
 }
 
 bool
 WebGLFramebuffer::ValidateForRead(const char* funcName,
                                   const webgl::FormatUsageInfo** const out_format,
                                   uint32_t* const out_width, uint32_t* const out_height)
 {
@@ -1310,18 +1318,16 @@ WebGLFramebuffer::GetAttachmentParameter
                                             " have different objects bound.",
                                             funcName);
             return JS::NullValue();
         }
 
         attachPoint = GetAttachPoint(LOCAL_GL_DEPTH_ATTACHMENT);
     }
 
-    FinalizeAttachments();
-
     return attachPoint->GetParameter(funcName, mContext, cx, target, attachment, pname,
                                      out_error);
 }
 
 
 void
 WebGLFramebuffer::GatherAttachments(std::vector<const WebGLFBAttachPoint*>* const out) const
 {
--- a/dom/canvas/WebGLFramebuffer.h
+++ b/dom/canvas/WebGLFramebuffer.h
@@ -96,17 +96,18 @@ public:
     bool HasUninitializedImageData() const;
     void SetImageDataStatus(WebGLImageDataStatus x);
 
     void Size(uint32_t* const out_width, uint32_t* const out_height) const;
 
     bool HasImage() const;
     bool IsComplete(WebGLContext* webgl, nsCString* const out_info) const;
 
-    void FinalizeAttachment(gl::GLContext* gl, GLenum attachmentLoc) const;
+    void FinalizeAttachment(gl::GLContext* gl, FBTarget target,
+                            GLenum attachmentLoc) const;
 
     JS::Value GetParameter(const char* funcName, WebGLContext* webgl, JSContext* cx,
                            GLenum target, GLenum attachment, GLenum pname,
                            ErrorResult* const out_error);
 
     void OnBackingStoreRespecified() const;
 };
 
@@ -224,17 +225,17 @@ public:
     void FramebufferTextureLayer(GLenum attachment, WebGLTexture* tex, GLint level,
                                  GLint layer);
 
     bool HasDefinedAttachments() const;
     bool HasIncompleteAttachments(nsCString* const out_info) const;
     bool AllImageRectsMatch() const;
     bool AllImageSamplesMatch() const;
     FBStatus PrecheckFramebufferStatus(nsCString* const out_info) const;
-    FBStatus CheckFramebufferStatus(nsCString* const out_info) const;
+    FBStatus CheckFramebufferStatus(FBTarget target, nsCString* const out_info) const;
 
     const webgl::FormatUsageInfo*
     GetFormatForAttachment(const WebGLFBAttachPoint& attachment) const;
 
     const WebGLFBAttachPoint& ColorAttachment(size_t colorAttachmentId) const {
         MOZ_ASSERT(colorAttachmentId < 1 + mMoreColorAttachments.Size());
         return colorAttachmentId ? mMoreColorAttachments[colorAttachmentId - 1]
                                  : mColorAttachment0;
@@ -267,17 +268,17 @@ public:
     void DetachTexture(const WebGLTexture* tex);
 
     void DetachRenderbuffer(const WebGLRenderbuffer* rb);
 
     WebGLContext* GetParentObject() const {
         return mContext;
     }
 
-    void FinalizeAttachments() const;
+    void FinalizeAttachments(FBTarget target) const;
 
     virtual JSObject* WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto) override;
 
     NS_INLINE_DECL_CYCLE_COLLECTING_NATIVE_REFCOUNTING(WebGLFramebuffer)
     NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_NATIVE_CLASS(WebGLFramebuffer)
 
     bool ValidateAndInitAttachments(const char* funcName);
 
--- a/dom/canvas/WebGLRenderbuffer.cpp
+++ b/dom/canvas/WebGLRenderbuffer.cpp
@@ -223,32 +223,30 @@ WebGLRenderbuffer::RenderbufferStorage(c
     mWidth = width;
     mHeight = height;
     mImageDataStatus = WebGLImageDataStatus::UninitializedImageData;
 
     InvalidateStatusOfAttachedFBs();
 }
 
 void
-WebGLRenderbuffer::DoFramebufferRenderbuffer(GLenum attachment) const
+WebGLRenderbuffer::DoFramebufferRenderbuffer(FBTarget target, GLenum attachment) const
 {
     gl::GLContext* gl = mContext->gl;
 
     if (attachment == LOCAL_GL_DEPTH_STENCIL_ATTACHMENT) {
         const GLuint stencilRB = (mSecondaryRB ? mSecondaryRB : mPrimaryRB);
-        gl->fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER,
-                                     LOCAL_GL_DEPTH_ATTACHMENT,
+        gl->fFramebufferRenderbuffer(target, LOCAL_GL_DEPTH_ATTACHMENT,
                                      LOCAL_GL_RENDERBUFFER, mPrimaryRB);
-        gl->fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER,
-                                     LOCAL_GL_STENCIL_ATTACHMENT,
+        gl->fFramebufferRenderbuffer(target, LOCAL_GL_STENCIL_ATTACHMENT,
                                      LOCAL_GL_RENDERBUFFER, stencilRB);
         return;
     }
 
-    gl->fFramebufferRenderbuffer(LOCAL_GL_FRAMEBUFFER, attachment,
+    gl->fFramebufferRenderbuffer(target, attachment,
                                  LOCAL_GL_RENDERBUFFER, mPrimaryRB);
 }
 
 GLint
 WebGLRenderbuffer::GetRenderbufferParameter(RBTarget target,
                                             RBParam pname) const
 {
     gl::GLContext* gl = mContext->gl;
--- a/dom/canvas/WebGLRenderbuffer.h
+++ b/dom/canvas/WebGLRenderbuffer.h
@@ -80,16 +80,16 @@ public:
 
     virtual JSObject* WrapObject(JSContext* cx, JS::Handle<JSObject*> givenProto) override;
 
 protected:
     ~WebGLRenderbuffer() {
         DeleteOnce();
     }
 
-    void DoFramebufferRenderbuffer(GLenum attachment) const;
+    void DoFramebufferRenderbuffer(FBTarget target, GLenum attachment) const;
     GLenum DoRenderbufferStorage(uint32_t samples, const webgl::FormatUsageInfo* format,
                                  uint32_t width, uint32_t height);
 };
 
 } // namespace mozilla
 
 #endif // WEBGL_RENDERBUFFER_H_