We should pass the conformance2 test 'multisampled-renderbuffer-initialization'.

Test result: failed: at (0, 12) expected: 0,0,0,0 was 0,0,0,255 failed: at (0, 12) expected: 0,0,0,0 was 0,0,0,255 failed: at (0, 12) expected: 0,0,0,0 was 0,0,0,255 failed: at (0, 12) expected: 0,0,0,0 was 0,0,0,255

The test failed due to the lack of RenderBuffer initialization. According to the spec, the WebGL implementation must initialize their contents to 0 when the initial data is not provided. So I clear the buffer value to zero in the RenderbufferStorage, and do a little fix for the ScopedFramebufferForRenderbuffer to keep the binding of framebuffer. [1] https://www.khronos.org/registry/webgl/specs/latest/1.0/#4.1

Comment on attachment 8699908 [details] [diff] [review] Initialize the renderbuffer Review of attachment 8699908 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +1677,5 @@ > ErrorName(error)); > return; > } > + > + // Initialize Renderbuffer to zero Don't eagerly do this. It should be lazily initialized like everything else. ::: dom/canvas/WebGLFramebufferAttachable.h @@ +19,5 @@ > // Track FBO/Attachment combinations > void MarkAttachment(const WebGLFBAttachPoint& attachment); > void UnmarkAttachment(const WebGLFBAttachPoint& attachment); > void InvalidateStatusOfAttachedFBs() const; > + bool HasAttachment(); IsAttached() ::: gfx/gl/ScopedGLHelpers.h @@ +235,5 @@ > > protected: > bool mComplete; // True if the framebuffer we create is complete. > GLuint mFB; > + ScopedBindFramebuffer mAutoFB; Why are you adding this?

Comment on attachment 8699908 [details] [diff] [review] Initialize the renderbuffer Review of attachment 8699908 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLContextGL.cpp @@ +1677,5 @@ > ErrorName(error)); > return; > } > + > + // Initialize Renderbuffer to zero okay, I will try to lazily initialize the Renderbuffer. ::: gfx/gl/ScopedGLHelpers.h @@ +235,5 @@ > > protected: > bool mComplete; // True if the framebuffer we create is complete. > GLuint mFB; > + ScopedBindFramebuffer mAutoFB; The autoFB was a local variable in the constructor, so it will bind old framebuffer after constructor. I want to keep the binding for initialization until destruct ScopedFramebufferForRenderbuffer.

Comment on attachment 8699908 [details] [diff] [review] Initialize the renderbuffer Review of attachment 8699908 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/gl/ScopedGLHelpers.h @@ +235,5 @@ > > protected: > bool mComplete; // True if the framebuffer we create is complete. > GLuint mFB; > + ScopedBindFramebuffer mAutoFB; We don't necessarily want this, as this changes the behavior of the function. We'd have to audit the call-sites to make sure this is a reasonable change, and also we should change this name to ScopedBindFramebufferForRenderbuffer. We should avoid doing this in this bug though. Open another bug if you'd like to work on this.

Comment on attachment 8709318 [details] [diff] [review] Initialize rb correctly if rb bind to READ_FRAMEBUFFER. Review of attachment 8709318 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGL2ContextRenderbuffers.cpp @@ +62,5 @@ > const char funcName[] = "renderbufferStorageMultisample"; > if (IsContextLost()) > return; > > + RenderbufferStorage_base(funcName, target, samples, internalFormat, width, height); This is bug 1094458. We can't just re-enable this yet. ::: dom/canvas/WebGLFramebuffer.cpp @@ +992,5 @@ > + { > + // This FB maybe bind to GL_READ_FRAMEBUFFER and glClear only > + // clear GL_DRAW_FRAMEBUFFER. So bind FB to GL_DRAW_FRAMEBUFFER > + // here. > + gl::ScopedBindFramebuffer autoFB(mContext->gl, mGLName); While I would normally object to rebinding unconditionally, this is an implicit clear, so we don't care whether it's fast. Therefore this is fine.