528013 - [webgl] bindFramebuffer checks target against wrong values

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Ilmari Heikkinen]

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

[Martin Weusten]

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

Comment 2

[Mark Steele]

Attachment: fix bindFramebuffer check

Comment 3

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

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..