1170893 - WebGL 2 - gl.getFramebufferAttachmentParameter causes MOZ_ASSERT to fire

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[u480271]

In UnsizedInternalFormatAndTypeFromEffectiveInternalFormat(...):

    MOZ_ASSERT(TypeFromInternalFormat(effectiveinternalformat) != LOCAL_GL_NONE);

Assertion failure: TypeFromInternalFormat(effectiveinternalformat) != 0, at /Users/djg/Mozilla/gecko/dom/canvas/WebGLContextUtils.cpp:177
#01: mozilla::UnsizedInternalFormatAndTypeFromEffectiveInternalFormat(StrongGLenum<TexInternalFormatDetails>, StrongGLenum<TexInternalFormatDetails>*, StrongGLenum<TexTypeDetails>*)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2c72fb1]
#02: mozilla::WebGLContext::GetFramebufferAttachmentParameter(JSContext*, unsigned int, unsigned int, unsigned int, mozilla::ErrorResult&)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2c4734f]
#03: mozilla::WebGLContext::GetFramebufferAttachmentParameter(JSContext*, unsigned int, unsigned int, unsigned int, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x270a2c1]
#04: mozilla::dom::WebGLRenderingContextBinding::getFramebufferAttachmentParameter(JSContext*, JS::Handle<JSObject*>, mozilla::WebGLContext*, JSJitMethodCallArgs const&)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2692b3c]
#05: mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2b7d064]
#06: js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5ffbfeb]
#07: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5f7ec4a]
#08: Interpret(JSContext*, js::RunState&)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5f9ac73]
#09: js::RunScript(JSContext*, js::RunState&)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5f8cefe]
#10: js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::ExecuteType, js::AbstractFramePtr, JS::Value*)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5fa854f]
#11: EvalKernel(JSContext*, JS::CallArgs const&, EvalType, js::AbstractFramePtr, JS::Handle<JSObject*>, unsigned char*)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5dc1218]
#12: js::DirectEval(JSContext*, JS::CallArgs const&)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5dc162d]
#13: Interpret(JSContext*, js::RunState&)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5f9a354]
#14: js::RunScript(JSContext*, js::RunState&)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5f8cefe]
#15: js::Invoke(JSContext*, JS::CallArgs, js::MaybeConstruct)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5f7ed8b]
#16: js::Invoke(JSContext*, JS::Value const&, JS::Value const&, unsigned int, JS::Value const*, JS::MutableHandle<JS::Value>)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x5f63dc4]
#17: js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>)[/Users/djg/Mozilla/gecko/objdir-desktop-debug.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x630eca7]

Comment 1

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

We're failing at [1] because effectiveInternalFormat is GL_NONE.

Looks like the TexImage2D call [2][3] fails with INVALID_OPERATION, and the effectiveInternalFormat doesn't get set at [4].

Debug output:
[KHR_debug: 0x7fffb678d000] ID 1282: SOURCE_API, TYPE_ERROR, SEVERITY_HIGH:
    GL_INVALID_OPERATION error generated. Cannot access buffer data.

Will have to track this down properly tomorrow.


[1] https://hg.mozilla.org/mozilla-central/file/c223b8844264/dom/canvas/WebGLContextGL.cpp#l1207
[2] https://github.com/KhronosGroup/WebGL/blob/d802ec17ea6604ef5346efd502f09ba435a57f11/sdk/tests/js/tests/gl-object-get-calls.js#L112
[3] https://hg.mozilla.org/mozilla-central/file/c223b8844264/dom/canvas/WebGLContextGL.cpp#l3227
[4] https://hg.mozilla.org/mozilla-central/file/c223b8844264/dom/canvas/WebGLContextGL.cpp#l3388

Comment 2

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

From [1]:
> GL_INVALID_OPERATION is generated if a non-zero buffer object name is bound to the
> GL_PIXEL_UNPACK_BUFFER target and the buffer object's data store is currently mapped.

Unbinding the buffer after the test at [2] lets glTexImage2D succeed, and gets us past this assertion.


[1] https://www.opengl.org/wiki/GLAPI/glTexImage2D
[2] https://github.com/KhronosGroup/WebGL/blob/d802ec17ea6604ef5346efd502f09ba435a57f11/sdk/tests/js/tests/gl-object-get-calls.js#L97

Comment 3

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 1170893-attach-point-assertion.patch

Also ran into an assertion in EnsureColorAttachPoints when given non-color attachment parameters.

Comment 4

[u480271]

Comment on attachment 8622858 [details] [diff] [review]
1170893-attach-point-assertion.patch

Review of attachment 8622858 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks for finding this.

::: dom/canvas/WebGLContextGL.cpp
@@ +1130,5 @@
>      }
>  
> +    if (IsExtensionEnabled(WebGLExtensionID::WEBGL_draw_buffers) &&
> +        attachment >= LOCAL_GL_COLOR_ATTACHMENT0 &&
> +        attachment <= LOCAL_GL_MAX_COLOR_ATTACHMENTS)

GL_MAX_COLOR_ATTACHMENTS is wrong. It's a name to pass to glGetIntegerv to ask for the maximum number of attachments supported by the GL.

What you want is attachment >= LOCAL_GL_COLOR_ATTACHMENT0 && attachment <= LOCAL_GL_COLOR_ATTACHMENT15.

Comment 5

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 1170893-attach-point-assertion.patch

Comment 6

[Edwin Flores [inactive from 2016-12-01] [:eflores] [:edwin]]

Attachment: 1170893-internalformat-check.patch

And add a check for the original crash. Unfortunately the test case passes (wrongly, AFAICT) after this patch, but *shrug*.

Comment 7

[u480271]

Comment on attachment 8622874 [details] [diff] [review]
1170893-internalformat-check.patch

Review of attachment 8622874 [details] [diff] [review]:
-----------------------------------------------------------------

R+ with nits fixed.

::: dom/canvas/WebGLContextGL.cpp
@@ +1213,5 @@
> +                    if (effectiveInternalFormat == LOCAL_GL_NONE) {
> +                        ErrorInvalidOperation("getFramebufferAttachmentParameter: "
> +                                              "texture contains no data");
> +                        return JS::NullValue();
> +                    }

This is correct, but a bit gross.

Looks like the entire switch should be skipped and an error reported if the bound texture doesn't have an effective internal format.

Please hoist the check above the switch.

Comment 8

[u480271]

Turns the test has quite a number of issues. I've fixed the stale UNPACK_BUFFER causing gl.texImage2D failure and submitted a PR against the conformance suite.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ab01e040db67

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ab01e040db67