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

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jerry, Assigned: jerry)

Tracking

unspecified
mozilla50
Points:
---

Firefox Tracking Flags

(firefox50 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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().
Created attachment 8773630 [details] [diff] [review]
handle gl.getFramebufferAttachmentParameter() call for default framebuffer. v1
Attachment #8773630 - Flags: review?(jgilbert)
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.
Attachment #8773630 - Flags: review?(jgilbert) → review+
Created attachment 8773777 [details] [diff] [review]
handle gl.getFramebufferAttachmentParameter() call for default framebuffer. v2. r=jgilbert

update
Attachment #8773777 - Flags: review+
Attachment #8773630 - Attachment is obsolete: true
please land the attachment 8773777 [details] [diff] [review] to m-c
Keywords: checkin-needed

Comment 6

2 years ago
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b005858abf58
Handle gl.getFramebufferAttachmentParameter() call for default framebuffer. r=jgilbert
Keywords: checkin-needed

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/b005858abf58
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.