The default bug view has changed. See this FAQ.

[webgl] bindFramebuffer checks target against wrong values

RESOLVED FIXED

Status

()

Core
Canvas: WebGL
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Ilmari Heikkinen, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Reporter)

Description

8 years ago
BindFramebuffer checks the target param against valid framebuffer attachments. This is completely wrong as the only valid target for BindFramebuffer is GL_FRAMEBUFFER.

http://www.khronos.org/opengles/sdk/docs/man/glBindFramebuffer.xml

The functions that take a framebuffer attachment value are FramebufferRenderbuffer and FramebufferTexture2D.

Further, the way framebuffers work is that you have only one framebuffer bound at any given time. All drawing goes to that framebuffer. The framebuffer then has attachments, which store the draw results. 

You can either attach textures using FramebufferTexture2D or renderbuffer objects with FramebufferRenderbuffer. Each attachment goes to one of the attachment targets: GL_COLOR_ATTACHMENT0, GL_DEPTH_ATTACHMENT, or GL_STENCIL_ATTACHMENT. Each attachment target can have only one object bound to it at any given time.

My understanding in JS terms:

Framebuffer.prototype = {
  colorAttachment : null,
  depthAttachment : null,
  stencilAttachment : null
}
Renderbuffer.prototype = {
  internalformat : null,
  width : null,
  height : null
}
bindFramebuffer = function(fbo){currentFbo = fbo}
bindRenderbuffer = function(rbo){currentRbo = rbo}
renderbufferStorage = function(internalformat, w, h){
  currentRbo.internalformat = internalformat;
  currentRbo.width = w;
  currentRbo.height = h;
}
framebufferTexture2D(attachment, tex){
  currentFbo[attachment] = tex;
}
framebufferRenderbuffer(attachment, rbo) {
  currentFbo[attachment] = rbo;
}


BindFramebuffer in WebGLContextGL.cpp:456
-    if (target >= LOCAL_GL_COLOR_ATTACHMENT0 &&
-        target < (LOCAL_GL_COLOR_ATTACHMENT0 + mBoundColorFramebuffers.Length()))
-    {
-        int targetOffset = target - LOCAL_GL_COLOR_ATTACHMENT0;
-        mBoundColorFramebuffers[targetOffset] = wfb;
-    } else if (target == LOCAL_GL_DEPTH_ATTACHMENT) {
-        mBoundDepthFramebuffer = wfb;
-    } else if (target == LOCAL_GL_STENCIL_ATTACHMENT) {
-        mBoundStencilFramebuffer = wfb;
-    } else {
+    if (target != LOCAL_GL_FRAMEBUFFER) {
         return ErrorMessage("glBindFramebuffer: invalid target");
     }
+    mBoundFramebuffer = wfb;
 
     gl->fBindFramebuffer(target, wfb ? wfb->GLName() : 0);

Comment 1

7 years ago
I can confirm this for Linux (x86_64) with build 2009-11-23.

Comment 2

7 years ago
Created attachment 415232 [details] [diff] [review]
fix bindFramebuffer check

Updated

7 years ago
Attachment #415232 - Flags: review?(vladimir)
Attachment #415232 - Flags: review?(vladimir) → review+
Good catch, checked in with some changes.

http://hg.mozilla.org/mozilla-central/rev/cf72d45dc256

We still need to track the framebuffer details at some point..
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.