Hit an assert for gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE). call stack: #01: Assertion failure: false (Should never happen.), at /Users/bignose/work/mozilla/gecko/git/mozilla_central_desktop/dom/canvas/WebGLFramebuffer.cpp:564 #02: mozilla::WebGLFBAttachPoint::GetParameter(char const*, mozilla::WebGLContext*, JSContext*, unsigned int, unsigned int, unsigned int, mozilla::ErrorResult*) (WebGLFramebuffer.cpp:564, in XUL) #03: mozilla::WebGLFramebuffer::GetAttachmentParameter(char const*, JSContext*, unsigned int, unsigned int, unsigned int, mozilla::ErrorResult*) (WebGLFramebuffer.cpp:1280, in XUL) #04: mozilla::WebGLContext::GetFramebufferAttachmentParameter(JSContext*, unsigned int, unsigned int, unsigned int, mozilla::ErrorResult&) (WebGLContextGL.cpp:753, in XUL) #05: mozilla::WebGL2Context::GetFramebufferAttachmentParameter(JSContext*, unsigned int, unsigned int, unsigned int, mozilla::ErrorResult&) (WebGL2ContextFramebuffers.cpp:368, in XUL) #06: mozilla::WebGLContext::GetFramebufferAttachmentParameter(JSContext*, unsigned int, unsigned int, unsigned int, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&) (WebGLContext.h:481, in XUL) #07: mozilla::dom::WebGLRenderingContextBinding::getFramebufferAttachmentParameter(JSContext*, JS::Handle<JSObject*>, mozilla::WebGLContext*, JSJitMethodCallArgs const&) (WebGLRenderingContextBinding.cpp:13030, in XUL) #08: mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*) (BindingUtils.cpp:2771, in XUL) #09: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) (jscntxtinlines.h:232, in XUL) #10: js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) (Interpreter.cpp:453, in XUL) #11: InternalCall(JSContext*, js::AnyInvokeArgs const&) (Interpreter.cpp:498, in XUL) #12: js::CallFromStack(JSContext*, JS::CallArgs const&) (Interpreter.cpp:504, in XUL) #13: Interpret(JSContext*, js::RunState&) (Interpreter.cpp:2873, in XUL) #14: js::RunScript(JSContext*, js::RunState&) (Interpreter.cpp:399, in XUL) #15: js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) (Interpreter.cpp:679, in XUL) #16: EvalKernel(JSContext*, JS::Handle<JS::Value>, EvalType, js::AbstractFramePtr, JS::Handle<JSObject*>, unsigned char*, JS::MutableHandle<JS::Value>) (Eval.cpp:333, in XUL) #17: js::DirectEval(JSContext*, JS::Handle<JS::Value>, JS::MutableHandle<JS::Value>) (Eval.cpp:451, in XUL)

webgl test page.

In gles 3.04 p.146 table 3.14 https://www.khronos.org/registry/gles/specs/3.0/es_spec_3.0.4.pdf It says that the d bit of d24s8 format is 24. When we query the component type for depth_attachment for d24s8 format texture, we got Special(DEPTH24_STENCIL8) component type. enum class ComponentType : uint8_t { None, Int, // RGBA32I UInt, // RGBA32UI, STENCIL_INDEX8 NormInt, // RGBA8_SNORM NormUInt, // RGBA8, DEPTH_COMPONENT16 Float, // RGBA32F Special, // DEPTH24_STENCIL8 }; Which type should we return for this 24bits type? gl.INT or gl.SIGNED_NORMALIZED? And where is the spec for the component type with 1bit, 5bit, 3, 5, 10, 24(for some special format GL_R3_G3_B2, GL_RGB5_A1, GL_RGB10_A2.. etc)?

Forget the 1, 2, 3, 5 and 10 bit question. We only have: DEPTH_COMPONENT16 DEPTH_COMPONENT24 DEPTH_COMPONENT32F DEPTH24_STENCIL8 DEPTH32F_STENCIL8

Try to handle the ComponentType::Special[1] format case for FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE query. [1] https://dxr.mozilla.org/mozilla-central/search?q=ComponentType%3A%3ASpecial&redirect=false

Comment on attachment 8773346 [details] [diff] [review] handle gl.getFramebufferAttachmentParameter() FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE query for DS format. v1 Review of attachment 8773346 [details] [diff] [review]: ----------------------------------------------------------------- Please check [1]. We set ComponentType::Special for d24s8 and d32fs8, but there is no implementation for ComponentType::Special type. [1] https://dxr.mozilla.org/mozilla-central/search?q=%22ComponentType%3A%3ASpecial%22&redirect=false ::: dom/canvas/WebGLFramebuffer.cpp @@ +572,5 @@ > + case 16: > + ret = LOCAL_GL_UNSIGNED_NORMALIZED; > + break; > + case 24: > + ret = LOCAL_GL_UNSIGNED_NORMALIZED; I'm not sure the d16 and d24 should be SIGNED_NORMALIZED, UNSIGNED_NORMALIZED or uint/int.

Comment on attachment 8773346 [details] [diff] [review] handle gl.getFramebufferAttachmentParameter() FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE query for DS format. v1 Review of attachment 8773346 [details] [diff] [review]: ----------------------------------------------------------------- Let's reuse our format tables. For DEPTH24_STENCIL8, query DEPTH_COMPONENT24 for depth, and STENCIL_INDEX8 for stencil. Branch similarly for DEPTH32F_STENCIL8.

update for comment 6.

Comment on attachment 8773373 [details] [diff] [review] handle gl.getFramebufferAttachmentParameter() FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE query for DS format. v2 Review of attachment 8773373 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLFramebuffer.cpp @@ +618,3 @@ > } > break; > + } I'm not sure the indent of the switch case braces pair. There is a variable "const webgl::FormatInfo* formatInfo" in this switch case.

Comment on attachment 8773373 [details] [diff] [review] handle gl.getFramebufferAttachmentParameter() FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE query for DS format. v2 Review of attachment 8773373 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/canvas/WebGLFramebuffer.cpp @@ +618,3 @@ > } > break; > + } The {} block starts at the level of the non-case statements, so this whole block would need to be indented. Just do it without the local: if (attachment == LOCAL_GL_DEPTH_ATTACHMENT) { switch (format->effectiveFormat) { case EffectiveFormat::DEPTH24_STENCIL8: format = webgl::GetFormat(EffectiveFormat::DEPTH_COMPONENT24); break;

update for comment 9. fix the typo for attachment name.

please land the attachment 8773566 [details] [diff] [review] to m-c try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0fa34325ecca

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a68a82726062 Handle gl.getFramebufferAttachmentParameter() FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE query for DS format. r=jgilbert