Closed Bug 528013 Opened 11 years ago Closed 11 years ago

[webgl] bindFramebuffer checks target against wrong values

Categories

(Core :: Canvas: WebGL, defect)

x86
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: ilmari.heikkinen, Unassigned)

Details

Attachments

(1 file)

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);
I can confirm this for Linux (x86_64) with build 2009-11-23.
Attachment #415232 - Flags: review?(vladimir)
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
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.