1288598 - [WebGL2] pass gl.getFramebufferAttachmentParameter() call for default framebuffer in gl-object-get-calls.html

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Jerry Shih[:jerry] (UTC+8) (inactive)]

There are some failed with default framebuffer in gl-object-get-calls.html

// test default framebuffer
gl.bindFramebuffer(gl.FRAMEBUFFER, null);

gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE)
gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE)
gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING)

This bug try to fix some error handling in getFramebufferAttachmentParameter().

Comment 1

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: handle gl.getFramebufferAttachmentParameter() call for default framebuffer. v1

Comment 2

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Comment on attachment 8773630 [details] [diff] [review]
handle gl.getFramebufferAttachmentParameter() call for default framebuffer. v1

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

If these change are matched to the spec, maybe we also need to check the depth/stencil setting for FB attachment case.

e.g.
https://hg.mozilla.org/mozilla-central/annotate/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/dom/canvas/WebGLFramebuffer.cpp#l463
https://hg.mozilla.org/mozilla-central/annotate/4c05938a64a7fde3ac2d7f4493aee1c5f2ad8a0a/dom/canvas/WebGLFramebuffer.cpp#l554

::: dom/canvas/WebGLContextGL.cpp
@@ +774,5 @@
>          return JS::NullValue();
>      }
>  
>      switch (pname) {
>      case LOCAL_GL_FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE:

This is for the case:

  //use default framebuffer without stencil.
  shouldBe('gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE)', 'gl.NONE');

----
Currently, we return LOCAL_GL_FRAMEBUFFER_DEFAULT for all case.
I check whether if we have depth or stencil. Return GL_NONE for the mismatch case.

gles3 spec:
https://www.khronos.org/opengles/sdk/docs/man3/html/glGetFramebufferAttachmentParameteriv.xhtml

@@ +808,5 @@
>          if (attachment == LOCAL_GL_BACK)
>              return JS::NumberValue(8);
>          return JS::NumberValue(0);
>  
>      case LOCAL_GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE:

These three are for test:
wtu.shouldGenerateGLError(gl, gl.INVALID_OPERATION, 'gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE)');


------
For ATTACHMENT_XXX_SIZE, if the default framebuffer doesn't have the corresponding buffer, return InvalidOperation() for that case.

@@ +840,3 @@
>          return JS::NumberValue(0);
>  
>      case LOCAL_GL_FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE:

This is for test:
wtu.shouldGenerateGLError(gl, gl.INVALID_OPERATION, 'gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE)');


-----
Return InvalidOperation() if the default framebuffer doesn't have the corresponding buffer.

@@ +855,2 @@
>  
>      case LOCAL_GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING:

Similar to previous one.

Comment 3

[Jerry Shih[:jerry] (UTC+8) (inactive)]

Attachment: handle gl.getFramebufferAttachmentParameter() call for default framebuffer. v2. r=jgilbert

update

Comment 4

[Jerry Shih[:jerry] (UTC+8) (inactive)]

try:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=0fa34325ecca

Comment 5

[Jerry Shih[:jerry] (UTC+8) (inactive)]

please land the attachment 8773777 [details] [diff] [review] to m-c

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b005858abf58
Handle gl.getFramebufferAttachmentParameter() call for default framebuffer. r=jgilbert

Comment 7

[Carsten Book [:Tomcat]]

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