Closed Bug 1236395 Opened 4 years ago Closed 4 years ago

[WebGL2] pass getFramebufferAttachmentParameter in gl-object-get-calls.html

Categories

(Core :: Canvas: WebGL, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: pchang, Assigned: pchang)

References

Details

Attachments

(1 file, 3 obsolete files)

Got error from https://www.khronos.org/registry/webgl/sdk/tests/conformance2/state/gl-object-get-calls.html?webglVersion=2&quiet=0

Test getFramebufferAttachmentParameter
PASS getError was expected value: NO_ERROR : 
PASS getError was expected value: NO_ERROR : 
PASS getError was expected value: NO_ERROR : 
FAIL gl.checkFramebufferStatus(gl.FRAMEBUFFER) should be 36053. Was 36054.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) is gl.TEXTURE
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) is texture
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL) is 0
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE) is 0
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_RED_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_GREEN_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_BLUE_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_LAYER) is 0
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) is gl.TEXTURE
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) is texture
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_LEVEL) is 0
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_CUBE_MAP_FACE) is 0
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_RED_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_GREEN_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_BLUE_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0 + ii, gl.FRAMEBUFFER_ATTACHMENT_TEXTURE_LAYER) is 0
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) is gl.RENDERBUFFER
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) is renderbuffer
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) is gl.RENDERBUFFER
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) is renderbuffer
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) is gl.RENDERBUFFER
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_NAME) is renderbuffer
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_DEPTH_SIZE) is non-zero.
FAIL gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE) should be non-zero. Was 0
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) is non-zero.
PASS getError was expected value: INVALID_OPERATION : after evaluating: gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE)
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) is non-zero.
PASS getFramebufferAttachmentParameter correctly handled invalid parameter enums
PASS getFramebufferAttachmentParameter correctly handled invalid target enums
PASS getFramebufferAttachmentParameter correctly handled invalid attachment enums
PASS gl.checkFramebufferStatus(gl.FRAMEBUFFER) is gl.FRAMEBUFFER_COMPLETE
FAIL gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.BACK, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) should be 33304 (of type number). Was null (of type object).
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) is gl.FRAMEBUFFER_DEFAULT
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) is gl.FRAMEBUFFER_DEFAULT
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.BACK, gl.FRAMEBUFFER_ATTACHMENT_RED_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.BACK, gl.FRAMEBUFFER_ATTACHMENT_GREEN_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.BACK, gl.FRAMEBUFFER_ATTACHMENT_BLUE_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.BACK, gl.FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH, gl.FRAMEBUFFER_ATTACHMENT_DEPTH_SIZE) is non-zero.
FAIL gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE) should be non-zero. Was 0
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.BACK, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.BACK, gl.FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH, gl.FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_COMPONENT_TYPE) is non-zero.
PASS gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING) is non-zero.
PASS getFramebufferAttachmentParameter correctly handled invalid parameter enums
PASS getFramebufferAttachmentParameter correctly handled invalid target enums
FAIL getFramebufferAttachmentParameter returned 33304 instead of null for invalid attachment enum: COLOR
PASS getError was expected value: NO_ERROR :
Blocks: 1236394
Assignee: nobody → howareyou322
Attached the patch to fix errors about getFramebufferAttachmentParameter.
With this patch, I still get the following error[1] about wrong stencil size.

Jeff, the stencil caps of WebGL is false by default based on WebGL spec and the test case doesn't enable stencil by default[2]. Do we need to enable stencil somewhere in gecko or the test case should enable stencil during testing?

[1]FAIL gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL, gl.FRAMEBUFFER_ATTACHMENT_STENCIL_SIZE) should be non-zero. Was 0
[2]https://github.com/KhronosGroup/WebGL/blob/master/sdk/tests/js/tests/gl-object-get-calls.js#L32
Attachment #8703485 - Flags: review?(jgilbert)
Jeff, there is another error related to FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE with GL_COLOR_ATTACHMENT0. In WebGL, it uses GL_BACK, but it uses GL_COLOR based on GLES spec. Any concern if I replace all GL_COLOR with GL_BACK in [1]?

[1]https://dxr.mozilla.org/mozilla-central/source/dom/canvas/WebGLContextGL.cpp#734
Attachment #8703485 - Flags: review?(jgilbert) → review+
Add needinfo for comment 2.
Flags: needinfo?(jgilbert)
(In reply to peter chang[:pchang][:peter] from comment #2)
> Jeff, there is another error related to FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE
> with GL_COLOR_ATTACHMENT0. In WebGL, it uses GL_BACK, but it uses GL_COLOR
> based on GLES spec. Any concern if I replace all GL_COLOR with GL_BACK in
> [1]?
> 
> [1]https://dxr.mozilla.org/mozilla-central/source/dom/canvas/WebGLContextGL.
> cpp#734

Can you show me where we spec _BACK instead of _COLOR in WebGL? We should really be following the GLES spec in this regard.
Flags: needinfo?(jgilbert) → needinfo?(howareyou322)
Jeff, you can find it from [1].


https://github.com/KhronosGroup/WebGL/blob/master/sdk/tests/js/tests/gl-object-get-calls.js#L244
Flags: needinfo?(howareyou322)
Flags: needinfo?(jgilbert)
(In reply to peter chang[:pchang][:peter] from comment #5)
> Jeff, you can find it from [1].
> 
> 
> https://github.com/KhronosGroup/WebGL/blob/master/sdk/tests/js/tests/gl-
> object-get-calls.js#L244

This is not the spec, this is the tests. The tests can be wrong. If it doesn't say it in the specs, the test is wrong.

I suspect then (unless you find a mention in the spec I missed) that the test here is just wrong. We only care about passing tests in as much as it shows we implement the spec properly.

If the test is wrong, we need to submit a PR to fix the test in the Khronos repo. I can do this if you do not want to.
Flags: needinfo?(jgilbert)
(In reply to Jeff Gilbert [:jgilbert] from comment #6)
> (In reply to peter chang[:pchang][:peter] from comment #5)
> > Jeff, you can find it from [1].
> > 
> > 
> > https://github.com/KhronosGroup/WebGL/blob/master/sdk/tests/js/tests/gl-
> > object-get-calls.js#L244
> 
> This is not the spec, this is the tests. The tests can be wrong. If it
> doesn't say it in the specs, the test is wrong.
> 
> I suspect then (unless you find a mention in the spec I missed) that the
> test here is just wrong. We only care about passing tests in as much as it
> shows we implement the spec properly.
> 
> If the test is wrong, we need to submit a PR to fix the test in the Khronos
> repo. I can do this if you do not want to.

Yes, you are right. I think WebGL is still followed OpenGL ES 3.0.4[1]. Then I should a PR to fix it.

[1]https://www.khronos.org/registry/webgl/specs/latest/2.0/#3.7.4
Address reviewer's suggestion and create PR in Khronos to fix test case.

https://github.com/KhronosGroup/WebGL/pull/1421
Attachment #8703485 - Attachment is obsolete: true
Attachment #8706225 - Flags: review+
Ok, checking the specs, the first patch was onto something. Those COLORs should be BACKs, but replaced, not in-addition-to.
To be clear, GetFramebufferAttachmentParameter takes only BACK, never COLOR.
(In reply to Jeff Gilbert [:jgilbert] from comment #9)
> Ok, checking the specs, the first patch was onto something. Those COLORs
> should be BACKs, but replaced, not in-addition-to.

(In reply to Jeff Gilbert [:jgilbert] from comment #10)
> To be clear, GetFramebufferAttachmentParameter takes only BACK, never COLOR.

Yes, I will update the patch soon.
Change GL_COLOR to GL_BACK based on GLES spec[1].


https://www.khronos.org/opengles/sdk/docs/man32/html/glGetFramebufferAttachmentParameteriv.xhtml
Attachment #8706225 - Attachment is obsolete: true
Attachment #8706727 - Flags: review+
Redo the removed space
Attachment #8706727 - Attachment is obsolete: true
Attachment #8706728 - Flags: review+
The stencil issue from comment 1 was fixed in below khronos github. 
https://github.com/KhronosGroup/WebGL/pull/1435

Ready to land attachment 8706728 [details] [diff] [review]
(In reply to peter chang[:pchang][:peter] from comment #16)
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=17d88c27f1d5

Didn't see fail items related to my patch. Ready to land code.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/09036a6752ba
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.