Closed Bug 1381610 Opened 7 years ago Closed 7 years ago

WebGL Conformance conformance/renderbuffers/framebuffer-object-attachment.html

Categories

(Core :: Graphics: CanvasWebGL, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: svargas, Assigned: svargas)

Details

Attachments

(1 file, 5 obsolete files)

Right now this test: 

https://www.khronos.org/registry/webgl/sdk/tests/conformance/renderbuffers/framebuffer-object-attachment.html?webglVersion=1&quiet=0

Fails at the bottom with: 

Test calling framebufferRenderbuffer before bindRenderbuffer.

Testing COLOR_ATTACHMENT0
FAIL getError expected: INVALID_OPERATION. Was NO_ERROR : bindRenderbuffer must be called before attachment to COLOR_ATTACHMENT0
FAIL gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) should be 0. Was 36161.
FAIL getError expected: INVALID_ENUM. Was NO_ERROR : Only OBJECT_TYPE can be queried when no image is attached

Testing DEPTH_ATTACHMENT
FAIL getError expected: INVALID_OPERATION. Was NO_ERROR : bindRenderbuffer must be called before attachment to DEPTH_ATTACHMENT
FAIL gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) should be 0. Was 36161.
FAIL getError expected: INVALID_ENUM. Was NO_ERROR : Only OBJECT_TYPE can be queried when no image is attached

Testing STENCIL_ATTACHMENT
FAIL getError expected: INVALID_OPERATION. Was NO_ERROR : bindRenderbuffer must be called before attachment to STENCIL_ATTACHMENT
FAIL gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) should be 0. Was 36161.
FAIL getError expected: INVALID_ENUM. Was NO_ERROR : Only OBJECT_TYPE can be queried when no image is attached

Testing DEPTH_STENCIL_ATTACHMENT
FAIL getError expected: INVALID_OPERATION. Was NO_ERROR : bindRenderbuffer must be called before attachment to DEPTH_STENCIL_ATTACHMENT
FAIL gl.getFramebufferAttachmentParameter(gl.FRAMEBUFFER, gl.DEPTH_STENCIL_ATTACHMENT, gl.FRAMEBUFFER_ATTACHMENT_OBJECT_TYPE) should be 0. Was 36161.
FAIL getError expected: INVALID_ENUM. Was NO_ERROR : Only OBJECT_TYPE can be queried when no image is attached
PASS successfullyParsed is true

Because framebufferRenderbuffer doesn't check to make sure bindRenderbuffer was previously called.
Forgot to terminate a string :-(

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2e09fe0b9a6feb335b90130b490cf0fa498a1b45
Attachment #8887204 - Attachment is obsolete: true
Attachment #8887204 - Flags: review?(jgilbert)
Attachment #8887224 - Flags: review?(jgilbert)
Comment on attachment 8887224 [details] [diff] [review]
0001-Bug-1381610-Check-bindRenderbuffer-called-before-fra.patch

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

::: dom/canvas/WebGLFramebuffer.cpp
@@ +1374,4 @@
>      MOZ_ASSERT(mContext->mBoundDrawFramebuffer == this ||
>                 mContext->mBoundReadFramebuffer == this);
>  
> +    if (rb && attachEnum && !mContext->IsRenderbuffer(rb)) {

This has to go below the `rb` validation below.

@@ +1374,5 @@
>      MOZ_ASSERT(mContext->mBoundDrawFramebuffer == this ||
>                 mContext->mBoundReadFramebuffer == this);
>  
> +    if (rb && attachEnum && !mContext->IsRenderbuffer(rb)) {
> +        mContext->ErrorInvalidOperation("FramebufferRenderbuffer: bindRenderbuffer must be called"

Use %s: and funcName.
Attachment #8887224 - Flags: review?(jgilbert) → review-
Attachment #8887224 - Attachment is obsolete: true
Attachment #8887258 - Flags: review?(jgilbert)
Comment on attachment 8887258 [details] [diff] [review]
0001-Bug-1381610-Check-bindRenderbuffer-called-before-fra.patch

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

::: dom/canvas/WebGLFramebuffer.cpp
@@ +1392,5 @@
> +    if (rb) {
> +        if (!mContext->ValidateObject("framebufferRenderbuffer: rb", *rb))
> +            return;
> +
> +        if (attachEnum && !mContext->IsRenderbuffer(rb)) {

Why do we need to check for attachEnum here?

@@ +1394,5 @@
> +            return;
> +
> +        if (attachEnum && !mContext->IsRenderbuffer(rb)) {
> +            mContext->ErrorInvalidOperation("%s: bindRenderbuffer must be called before attachment "
> +                                            "to %04x", "framebufferRenderbuffer", attachEnum);

Put the trailing space in the quotes on the next line, not the end of the previous.

Also when spilling args, don't stack more args on the tail line after the comma:
if ("foo bar"
    " bax",
    more, args)
Attachment #8887258 - Attachment is obsolete: true
Attachment #8887258 - Flags: review?(jgilbert)
Attachment #8887260 - Flags: review?(jgilbert)
-        if (!mContext->IsRenderbuffer(rb)) {
+        if (!rb->mHasBeenBound) {
Attachment #8887260 - Attachment is obsolete: true
Attachment #8887260 - Flags: review?(jgilbert)
Attachment #8887264 - Flags: review?(jgilbert)
Comment on attachment 8887264 [details] [diff] [review]
0001-Bug-1381610-Check-bindRenderbuffer-called-before-fra.patch

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

::: dom/canvas/WebGLFramebuffer.cpp
@@ +1395,5 @@
> +
> +        if (!rb->mHasBeenBound) {
> +            mContext->ErrorInvalidOperation("%s: bindRenderbuffer must be called before"
> +                                            " attachment to %04x",
> +                                            "framebufferRenderbuffer", attachEnum);

s/"framebufferRenderbuffer"/funcName/
Attachment #8887264 - Flags: review?(jgilbert) → review+
"framebufferRenderbuffer" -> funcName
Attachment #8887264 - Attachment is obsolete: true
Attachment #8887265 - Flags: review?(jgilbert)
Keywords: checkin-needed
Attachment #8887265 - Flags: review?(jgilbert) → review+
Pushed by ryanvm@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/28836e42d21a
Check bindRenderbuffer called before framebufferRenderbuffer. r=jgilbert
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/28836e42d21a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: