Robustify GL FBO framebuffer properties handling

panda/src/cocoadisplay/cocoaGraphicsPipe.mm

@@ -324,34 +324,31 @@ make_output(const string &name,
   }
 
   // Second thing to try: a GLGraphicsBuffer
+
   if (retry == 1) {
-    if ((host==0)||
+    if (!gl_support_fbo || host == NULL ||
-  //        (!gl_support_fbo)||
+        (flags & (BF_require_parasite | BF_require_window)) != 0) {
-        ((flags&BF_require_parasite)!=0)||
-        ((flags&BF_require_window)!=0)) {
       return NULL;
     }
     // Early failure - if we are sure that this buffer WONT
     // meet specs, we can bail out early.
-    if ((flags & BF_fb_props_optional)==0) {
+    if ((flags & BF_fb_props_optional) == 0) {
-      if ((fb_prop.get_indexed_color() > 0)||
+      if (fb_prop.get_indexed_color() ||
-          (fb_prop.get_back_buffers() > 0)||
+          fb_prop.get_back_buffers() > 0 ||
-          (fb_prop.get_accum_bits() > 0)||
+          fb_prop.get_accum_bits() > 0) {
-          (fb_prop.get_multisamples() > 0)) {
         return NULL;
       }
     }
+    if (cocoagsg != NULL && cocoagsg->is_valid() && !cocoagsg->needs_reset()) {
+      if (!cocoagsg->_supports_framebuffer_object ||
+          cocoagsg->_glDrawBuffers == NULL) {
+        return NULL;
+      } else {
         // Early success - if we are sure that this buffer WILL
         // meet specs, we can precertify it.
-    if ((cocoagsg != 0) &&
-        (cocoagsg->is_valid()) &&
-        (!cocoagsg->needs_reset()) &&
-        (cocoagsg->_supports_framebuffer_object) &&
-        (cocoagsg->_glDrawBuffers != 0) &&
-        (fb_prop.is_basic())) {
         precertify = true;
       }
-
+    }
     return new GLGraphicsBuffer(engine, this, name, fb_prop, win_prop,
                                 flags, gsg, host);
   }

panda/src/display/config_display.cxx

@@ -418,19 +418,13 @@ ConfigVariableInt depth_bits
 ("depth-bits", 0,
  PRC_DESC("The minimum number of depth buffer bits requested."));
 ConfigVariableInt color_bits
-("color-bits", 0,
+("color-bits", "",
- PRC_DESC("The minimum number of total color buffer bits requested.  This "
+ PRC_DESC("The minimum number of total color buffer bits requested.  If you "
-          "value is like red-bits + blue-bits + green-bits except Panda "
+          "specify only one value, it will represent the total value for the "
-          "won't care how the bits are divided up."));
+          "red, green and blue channels, and indicates you don't care how the "
-ConfigVariableInt red_bits
+          "bits are divided up among the red, green and blue channels.  If "
-("red-bits", 0,
+          "you specify three values, it represents three separate red, green "
- PRC_DESC("The minimum number of red color buffer bits requested."));
+          "and blue bit requirements."));
-ConfigVariableInt green_bits
-("green-bits", 0,
- PRC_DESC("The minimum number of green color buffer bits requested."));
-ConfigVariableInt blue_bits
-("blue-bits", 0,
- PRC_DESC("The minimum number of blue color buffer bits requested."));
 ConfigVariableInt alpha_bits
 ("alpha-bits", 0,
  PRC_DESC("The minimum number of alpha buffer bits requested."));

panda/src/display/config_display.h

@@ -97,9 +97,6 @@ extern EXPCL_PANDA_DISPLAY ConfigVariableBool framebuffer_srgb;
 extern EXPCL_PANDA_DISPLAY ConfigVariableBool framebuffer_float;
 extern EXPCL_PANDA_DISPLAY ConfigVariableInt depth_bits;
 extern EXPCL_PANDA_DISPLAY ConfigVariableInt color_bits;
-extern EXPCL_PANDA_DISPLAY ConfigVariableInt red_bits;
-extern EXPCL_PANDA_DISPLAY ConfigVariableInt green_bits;
-extern EXPCL_PANDA_DISPLAY ConfigVariableInt blue_bits;
 extern EXPCL_PANDA_DISPLAY ConfigVariableInt alpha_bits;
 extern EXPCL_PANDA_DISPLAY ConfigVariableInt stencil_bits;
 extern EXPCL_PANDA_DISPLAY ConfigVariableInt accum_bits;

panda/src/display/frameBufferProperties.I

@@ -305,37 +305,42 @@ get_float_depth() const {
 INLINE void FrameBufferProperties::
 set_depth_bits(int n) {
   _property[FBP_depth_bits] = n;
-  _specified[FBP_depth_bits] = true;
+  _specified |= (1 << FBP_depth_bits);
 }
 
 ////////////////////////////////////////////////////////////////////
 //     Function: FrameBufferProperties::set_color_bits
 //       Access: Published
-//  Description:
+//  Description: Sets the number of requested color bits as a single
+//               number that represents the sum of the individual
+//               numbers of red, green and blue bits.  Panda won't
+//               care how the individual bits are divided up.
+//
+//               See also set_rgba_bits, which allows you to specify
+//               requirements for the individual components.
 ////////////////////////////////////////////////////////////////////
 INLINE void FrameBufferProperties::
 set_color_bits(int n) {
   _property[FBP_color_bits] = n;
-  _specified[FBP_color_bits] = true;
+  _specified |= (1 << FBP_color_bits);
 }
 
 ////////////////////////////////////////////////////////////////////
 //     Function: FrameBufferProperties::set_rgba_bits
 //       Access: Published
-//  Description: Sets all color bit requirements separately.
+//  Description: Convenience method for setting the red, green, blue
+//               and alpha bits in one go.
 ////////////////////////////////////////////////////////////////////
 INLINE void FrameBufferProperties::
 set_rgba_bits(int r, int g, int b, int a) {
   _property[FBP_red_bits] = r;
-  _specified[FBP_red_bits] = true;
   _property[FBP_green_bits] = g;
-  _specified[FBP_green_bits] = true;
   _property[FBP_blue_bits] = b;
-  _specified[FBP_blue_bits] = true;
   _property[FBP_alpha_bits] = a;
-  _specified[FBP_alpha_bits] = true;
   _property[FBP_color_bits] = r + g + b;
-  _specified[FBP_color_bits] = true;
+  _specified |= (1 << FBP_color_bits) | (1 << FBP_red_bits) |
+                (1 << FBP_green_bits) | (1 << FBP_blue_bits) |
+                (1 << FBP_alpha_bits);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -346,7 +351,7 @@ set_rgba_bits(int r, int g, int b, int a) {
 INLINE void FrameBufferProperties::
 set_red_bits(int n) {
   _property[FBP_red_bits] = n;
-  _specified[FBP_red_bits] = true;
+  _specified |= (1 << FBP_red_bits);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -357,7 +362,7 @@ set_red_bits(int n) {
 INLINE void FrameBufferProperties::
 set_green_bits(int n) {
   _property[FBP_green_bits] = n;
-  _specified[FBP_green_bits] = true;
+  _specified |= (1 << FBP_green_bits);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -368,7 +373,7 @@ set_green_bits(int n) {
 INLINE void FrameBufferProperties::
 set_blue_bits(int n) {
   _property[FBP_blue_bits] = n;
-  _specified[FBP_blue_bits] = true;
+  _specified |= (1 << FBP_blue_bits);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -379,7 +384,7 @@ set_blue_bits(int n) {
 INLINE void FrameBufferProperties::
 set_alpha_bits(int n) {
   _property[FBP_alpha_bits] = n;
-  _specified[FBP_alpha_bits] = true;
+  _specified |= (1 << FBP_alpha_bits);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -390,7 +395,7 @@ set_alpha_bits(int n) {
 INLINE void FrameBufferProperties::
 set_stencil_bits(int n) {
   _property[FBP_stencil_bits] = n;
-  _specified[FBP_stencil_bits] = true;
+  _specified |= (1 << FBP_stencil_bits);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -401,7 +406,7 @@ set_stencil_bits(int n) {
 INLINE void FrameBufferProperties::
 set_accum_bits(int n) {
   _property[FBP_accum_bits] = n;
-  _specified[FBP_accum_bits] = true;
+  _specified |= (1 << FBP_accum_bits);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -413,7 +418,7 @@ INLINE void FrameBufferProperties::
 set_aux_rgba(int n) {
   nassertv(n < 4);
   _property[FBP_aux_rgba] = n;
-  _specified[FBP_aux_rgba] = true;
+  _specified |= (1 << FBP_aux_rgba);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -425,7 +430,7 @@ INLINE void FrameBufferProperties::
 set_aux_hrgba(int n) {
   nassertv(n < 4);
   _property[FBP_aux_hrgba] = n;
-  _specified[FBP_aux_hrgba] = true;
+  _specified |= (1 << FBP_aux_hrgba);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -437,7 +442,7 @@ INLINE void FrameBufferProperties::
 set_aux_float(int n) {
   nassertv(n < 4);
   _property[FBP_aux_float] = n;
-  _specified[FBP_aux_float] = true;
+  _specified |= (1 << FBP_aux_float);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -448,7 +453,7 @@ set_aux_float(int n) {
 INLINE void FrameBufferProperties::
 set_multisamples(int n) {
   _property[FBP_multisamples] = n;
-  _specified[FBP_multisamples] = true;
+  _specified |= (1 << FBP_multisamples);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -460,7 +465,7 @@ set_multisamples(int n) {
 INLINE void FrameBufferProperties::
 set_coverage_samples(int n) {
   _property[FBP_coverage_samples] = n;
-  _specified[FBP_coverage_samples] = true;
+  _specified |= (1 << FBP_coverage_samples);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -471,7 +476,7 @@ set_coverage_samples(int n) {
 INLINE void FrameBufferProperties::
 set_back_buffers(int n) {
   _property[FBP_back_buffers] = n;
-  _specified[FBP_back_buffers] = true;
+  _specified |= (1 << FBP_back_buffers);
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/display/frameBufferProperties.cxx

@@ -37,9 +37,9 @@ void FrameBufferProperties::
 operator = (const FrameBufferProperties &copy) {
   _flags_specified = copy._flags_specified;
   _flags = copy._flags;
+  _specified = copy._specified;
 
   for (int i = 0; i < FBP_COUNT; ++i) {
-    _specified[i] = copy._specified[i];
     _property[i]  = copy._property[i];
   }
 }
@@ -144,17 +144,24 @@ get_default() {
   if (depth_bits > 0) {
     default_props.set_depth_bits(depth_bits);
   }
-  if (color_bits > 0) {
+  switch (color_bits.size()) {
-    default_props.set_color_bits(color_bits);
+  case 0:
-  }
+    break;
-  if (red_bits > 0) {
+  case 1:
-    default_props.set_red_bits(red_bits);
+    default_props.set_color_bits(color_bits[0]);
-  }
+    break;
-  if (green_bits > 0) {
+  case 3:
-    default_props.set_green_bits(green_bits);
+    default_props.set_color_bits(color_bits[0] + color_bits[1] + color_bits[2]);
-  }
+    default_props.set_red_bits(color_bits[0]);
-  if (blue_bits > 0) {
+    default_props.set_green_bits(color_bits[1]);
-    default_props.set_blue_bits(blue_bits);
+    default_props.set_blue_bits(color_bits[2]);
+    break;
+  default:
+    default_props.set_color_bits(color_bits[0]);
+    display_cat.error()
+      << "Configuration variable color-bits takes either 1 or 3 values, not "
+      << color_bits.size() << "\n";
+    break;
   }
   if (alpha_bits > 0) {
     default_props.set_alpha_bits(alpha_bits);
@@ -189,10 +196,11 @@ operator == (const FrameBufferProperties &other) const {
     return false;
   }
 
-  for (int i = 0; i < FBP_COUNT; ++i) {
+  if (_specified != other._specified) {
-    if (_specified[i] != other._specified[i]) {
     return false;
   }
+
+  for (int i = 0; i < FBP_COUNT; ++i) {
     if (_property[i] != other._property[i]) {
       return false;
     }
@@ -214,9 +222,9 @@ clear() {
   _flags_specified = 0;
 
   for (int i = 0; i < FBP_COUNT; ++i) {
-    _specified[i] = 0;
     _property[i] = 0;
   }
+  _specified = 0;
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -232,9 +240,9 @@ add_properties(const FrameBufferProperties &other) {
   _flags |= other._flags & other._flags_specified;
 
   for (int i = 0; i < FBP_COUNT; ++i) {
-    if (other._specified[i]) {
+    if (other._specified & (1 << i)) {
       _property[i] = other._property[i];
-      _specified[i] = true;
+      _specified |= (1 << i);
     }
   }
 }
@@ -364,30 +372,18 @@ get_buffer_mask() const {
 ////////////////////////////////////////////////////////////////////
 bool FrameBufferProperties::
 is_any_specified() const {
-  if (_flags_specified != 0) {
+  return (_flags_specified | _specified) != 0;
-    return true;
-  }
-
-  for (int i = 0; i < FBP_COUNT; ++i) {
-    if (_specified[i]) {
-      return true;
-    }
-  }
-  return false;
 }
 
 ////////////////////////////////////////////////////////////////////
 //     Function: FrameBufferProperties::set_all_specified
 //       Access: Published
-//  Description: sets all the specified bits.
+//  Description: Marks all bits as having been specified.
 ////////////////////////////////////////////////////////////////////
 void FrameBufferProperties::
 set_all_specified() {
   _flags_specified = FBF_all;
-
+  _specified = (1 << FBP_COUNT) - 1;
-  for (int i = 0; i < FBP_COUNT; ++i) {
-    _specified[i] = true;
-  }
 }
 
 ////////////////////////////////////////////////////////////////////

panda/src/display/frameBufferProperties.h

@@ -67,7 +67,7 @@ private:
   };
 
   int _property[FBP_COUNT];
-  bool _specified[FBP_COUNT];
+  int _specified;
 
   int _flags;
   int _flags_specified;

panda/src/glstuff/glGraphicsBuffer_src.cxx

@@ -30,10 +30,10 @@ CLP(GraphicsBuffer)(GraphicsEngine *engine, GraphicsPipe *pipe,
   GraphicsBuffer(engine, pipe, name, fb_prop, win_prop, flags, gsg, host),
   _bind_texture_pcollector(_draw_window_pcollector, "Bind textures"),
   _generate_mipmap_pcollector(_draw_window_pcollector, "Generate mipmaps"),
-  _resolve_multisample_pcollector(_draw_window_pcollector, "Resolve multisamples")
+  _resolve_multisample_pcollector(_draw_window_pcollector, "Resolve multisamples"),
+  _requested_multisamples(0),
+  _requested_coverage_samples(0)
 {
-  CLP(GraphicsStateGuardian) *glgsg;
-
   // A FBO doesn't have a back buffer.
   _draw_buffer_type       = RenderBuffer::T_front;
   _screenshot_buffer_type = RenderBuffer::T_front;
@@ -42,36 +42,6 @@ CLP(GraphicsBuffer)(GraphicsEngine *engine, GraphicsPipe *pipe,
   _fbo_multisample = 0;
   _initial_clear = true;
   _needs_rebuild = true;
-  DCAST_INTO_V(glgsg, _gsg);
-
-  if (glgsg->get_supports_framebuffer_multisample() && glgsg->get_supports_framebuffer_blit()) {
-    _requested_multisamples = fb_prop.get_multisamples();
-  } else {
-    _requested_multisamples = 0;
-  }
-
-  if (glgsg->get_supports_framebuffer_multisample_coverage_nv() && glgsg->get_supports_framebuffer_blit()) {
-    _requested_coverage_samples = fb_prop.get_coverage_samples();
-    // Note:  Only 4 and 8 actual samples are supported by the extension, with 8 or 16 coverage samples.
-    if ((_requested_coverage_samples <= 8) && (_requested_coverage_samples > 0)) {
-      _requested_multisamples = 4;
-      _requested_coverage_samples = 8;
-    } else if (_requested_coverage_samples > 8) {
-      if (_requested_multisamples < 8) {
-        _requested_multisamples = 4;
-      } else {
-        _requested_multisamples = 8;
-      }
-      _requested_coverage_samples = 16;
-    }
-
-  } else {
-    _requested_coverage_samples = 0;
-  }
-
-  if (_requested_multisamples > glgsg->_max_fb_samples) {
-    _requested_multisamples = glgsg->_max_fb_samples;
-  }
 
   _rb_size_x = 0;
   _rb_size_y = 0;
@@ -382,9 +352,9 @@ rebuild_bitplanes() {
     }
   }
 
-  // Decide whether we should use a depth stencil or just a regular depth attachment.
+  // Decide whether we should use a depth stencil or just a regular depth
-  // If nothing was attached to either RTP_depth_stencil or RTP_depth, we use a
+  // attachment.  If nothing was attached to either RTP_depth_stencil or
-  // depth-stencil renderbuffer.
+  // RTP_depth, we use a depth-stencil renderbuffer.
   _use_depth_stencil = false;
   if (_gsg->get_supports_depth_stencil()) {
     if (attach[RTP_depth_stencil]) {
@@ -396,19 +366,16 @@ rebuild_bitplanes() {
       // explicitly bound something to RTP_depth.
       _use_depth_stencil = false;
 
-    } else if (_fb_properties.get_float_depth()) {
-      // Let's not bother with a depth-stencil buffer
-      // if a float buffer was requested.
-      _use_depth_stencil = false;
-
-    } else if (_fb_properties.get_depth_bits() > 24) {
-      // We can't give more than 24 depth bits with a depth-stencil buffer.
-      _use_depth_stencil = false;
-
     } else if (_fb_properties.get_stencil_bits() > 0) {
       // The user requested stencil bits.  Let's take the hint.
       _use_depth_stencil = true;
 
+    } else if (_fb_properties.get_depth_bits() > 24 ||
+               _fb_properties.get_float_depth()) {
+      // 32-bit float depth is supported in conjunction with depth stencil,
+      // but it's a waste.  Let's not do it unless the user requested stencil.
+      _use_depth_stencil = false;
+
     } else if (_fb_properties.get_depth_bits() > 0) {
       // Let's use a depth stencil buffer by default, if a depth
       // buffer was requested.
@@ -416,10 +383,9 @@ rebuild_bitplanes() {
     }
   }
 
-  // Knowing this, we can already be a tiny bit
+  // Knowing this, we can already be a tiny bit more accurate about the
-  // more accurate about the framebuffer properties.
+  // framebuffer properties.
   if (_use_depth_stencil) {
-    _fb_properties.set_depth_bits(24);
     _fb_properties.set_stencil_bits(8);
   } else {
     _fb_properties.set_stencil_bits(0);
@@ -553,8 +519,7 @@ rebuild_bitplanes() {
 #endif  // OPENGLES
 
   if (!_have_any_color) {
-    _fb_properties.set_color_bits(0);
+    _fb_properties.set_rgba_bits(0, 0, 0, 0);
-    _fb_properties.set_alpha_bits(0);
   }
 
   _initial_clear = false;
@@ -699,8 +664,7 @@ bind_slot(int layer, bool rb_resize, Texture **attach, RenderTexturePlane slot,
         glGetTexLevelParameteriv(target, 0, GL_TEXTURE_BLUE_SIZE, &blue_size);
         glGetTexLevelParameteriv(target, 0, GL_TEXTURE_ALPHA_SIZE, &alpha_size);
 
-        _fb_properties.set_color_bits(red_size + green_size + blue_size);
+        _fb_properties.set_rgba_bits(red_size, green_size, blue_size, alpha_size);
-        _fb_properties.set_alpha_bits(alpha_size);
       }
 #endif
     }
@@ -717,6 +681,7 @@ bind_slot(int layer, bool rb_resize, Texture **attach, RenderTexturePlane slot,
     // No texture to bind.  Instead, create a renderbuffer.
     // Choose a suitable renderbuffer format based on the requirements.
 #ifdef OPENGLES
+    // OpenGL ES case.
     GLuint gl_format = GL_RGBA4;
     switch (slot) {
     case RTP_depth_stencil:
@@ -731,6 +696,7 @@ bind_slot(int layer, bool rb_resize, Texture **attach, RenderTexturePlane slot,
         gl_format = GL_DEPTH_COMPONENT16;
       }
       break;
+    //NB: we currently use RTP_stencil to store the right eye for stereo.
     //case RTP_stencil:
     //  gl_format = GL_STENCIL_INDEX8;
     //  break
@@ -762,7 +728,8 @@ bind_slot(int layer, bool rb_resize, Texture **attach, RenderTexturePlane slot,
     GLuint gl_format = GL_RGBA;
     switch (slot) {
       case RTP_depth_stencil:
-        if (_fb_properties.get_float_depth()) {
+        if (_fb_properties.get_depth_bits() > 24 ||
+            _fb_properties.get_float_depth()) {
           gl_format = GL_DEPTH32F_STENCIL8;
         } else {
           gl_format = GL_DEPTH24_STENCIL8;
@@ -775,7 +742,7 @@ bind_slot(int layer, bool rb_resize, Texture **attach, RenderTexturePlane slot,
           gl_format = GL_DEPTH_COMPONENT32;
         } else if (_fb_properties.get_depth_bits() > 16) {
           gl_format = GL_DEPTH_COMPONENT24;
-        } else if (_fb_properties.get_depth_bits() > 8) {
+        } else if (_fb_properties.get_depth_bits() > 1) {
           gl_format = GL_DEPTH_COMPONENT16;
         } else {
           gl_format = GL_DEPTH_COMPONENT;
@@ -903,8 +870,7 @@ bind_slot(int layer, bool rb_resize, Texture **attach, RenderTexturePlane slot,
         glgsg->_glGetRenderbufferParameteriv(GL_RENDERBUFFER_EXT, GL_RENDERBUFFER_GREEN_SIZE_EXT, &green_size);
         glgsg->_glGetRenderbufferParameteriv(GL_RENDERBUFFER_EXT, GL_RENDERBUFFER_BLUE_SIZE_EXT, &blue_size);
         glgsg->_glGetRenderbufferParameteriv(GL_RENDERBUFFER_EXT, GL_RENDERBUFFER_ALPHA_SIZE_EXT, &alpha_size);
-        _fb_properties.set_color_bits(red_size + green_size + blue_size);
+        _fb_properties.set_rgba_bits(red_size, green_size, blue_size, alpha_size);
-        _fb_properties.set_alpha_bits(alpha_size);
       }
       glgsg->_glBindRenderbuffer(GL_RENDERBUFFER_EXT, 0);
       glgsg->_glFramebufferRenderbuffer(GL_FRAMEBUFFER_EXT, attachpoint,
@@ -976,7 +942,6 @@ bind_slot_multisample(bool rb_resize, Texture **attach, RenderTexturePlane slot,
             format = GL_DEPTH_COMPONENT32;
             break;
           default:
-            format = GL_DEPTH_COMPONENT;
             break;
         }
       }
@@ -1269,14 +1234,12 @@ open_buffer() {
 
   // Describe the framebuffer properties of the FBO.
   //
-  // Unfortunately, we can't possibly predict which formats
+  // Unfortunately, we can't currently predict which formats
   // the implementation will allow us to use at this point, so
   // we'll just have to make some guesses and parrot the rest
   // of the properties back to the user.
   // When we actually attach the textures, we'll update the
   // properties more appropriately.
-  // This is probably safe, as we can usually bind just about
-  // any supported texture format to the FBO.
 
   // Rounding the depth bits is not spectacular, but at least we're
   // telling the user *something* about what we're going to get.
@@ -1286,6 +1249,9 @@ open_buffer() {
   if (_fb_properties.get_color_bits() == 0 &&
       _fb_properties.get_rgb_color()) {
     _fb_properties.set_color_bits(1);
+    _fb_properties.set_red_bits(1);
+    _fb_properties.set_green_bits(1);
+    _fb_properties.set_blue_bits(1);
   }
 
   // Actually, let's always get a colour buffer for now until we
@@ -1294,18 +1260,28 @@ open_buffer() {
     _fb_properties.set_color_bits(1);
   }
 
-  if (_fb_properties.get_depth_bits() >= 32) {
+  if (_fb_properties.get_depth_bits() > 24) {
     _fb_properties.set_depth_bits(32);
   } else if (_fb_properties.get_depth_bits() > 16) {
     _fb_properties.set_depth_bits(24);
-  } else if (_fb_properties.get_depth_bits() > 8) {
+  } else {
     _fb_properties.set_depth_bits(16);
   }
 
-  // We're not going to get more than this, ever.
+  // We're not going to get more than this, ever.  At least not until OpenGL
+  // introduces 64-bit texture formats.
   if (_fb_properties.get_color_bits() > 96) {
     _fb_properties.set_color_bits(96);
   }
+  if (_fb_properties.get_red_bits() > 32) {
+    _fb_properties.set_red_bits(32);
+  }
+  if (_fb_properties.get_green_bits() > 32) {
+    _fb_properties.set_green_bits(32);
+  }
+  if (_fb_properties.get_blue_bits() > 32) {
+    _fb_properties.set_blue_bits(32);
+  }
   if (_fb_properties.get_alpha_bits() > 32) {
     _fb_properties.set_alpha_bits(32);
   }
@@ -1317,30 +1293,63 @@ open_buffer() {
 
   // We currently only support color formats this big as float.
   if (_fb_properties.get_color_bits() > 16 * 3) {
-    _fb_properties.set_color_bits(32 * 3);
     _fb_properties.set_float_color(true);
-
-    if (_fb_properties.get_alpha_bits() > 0) {
-      _fb_properties.set_alpha_bits(32);
-    }
   }
 
   if (_fb_properties.get_srgb_color()) {
-    _fb_properties.set_color_bits(24);
+    // This is the only sRGB color format OpenGL supports.
+    _fb_properties.set_rgba_bits(8, 8, 8,
+      (_fb_properties.get_alpha_bits() > 0) ? 8 : 0);
     _fb_properties.set_float_color(false);
-
-    if (_fb_properties.get_alpha_bits() > 0) {
-      _fb_properties.set_alpha_bits(32);
-    }
   }
 
   if (!_gsg->get_supports_depth_stencil()) {
     // At least we know we won't be getting stencil bits.
     _fb_properties.set_stencil_bits(0);
+
+  } else if (_fb_properties.get_stencil_bits() > 0) {
+    // We don't currently support stencil-only targets.
+    _fb_properties.set_stencil_bits(8);
+    if (_fb_properties.get_depth_bits() < 24) {
+      _fb_properties.set_depth_bits(24);
     }
+  }
+
+  // Accumulation buffers aren't supported for FBOs.
   _fb_properties.set_accum_bits(0);
 
+  if (glgsg->get_supports_framebuffer_multisample() && glgsg->get_supports_framebuffer_blit()) {
+    _requested_multisamples = _fb_properties.get_multisamples();
+  } else {
+    _requested_multisamples = 0;
+  }
+
+#ifndef OPENGLES
+  if (glgsg->get_supports_framebuffer_multisample_coverage_nv() && glgsg->get_supports_framebuffer_blit()) {
+    _requested_coverage_samples = _fb_properties.get_coverage_samples();
+    // Note:  Only 4 and 8 actual samples are supported by the extension, with 8 or 16 coverage samples.
+    if ((_requested_coverage_samples <= 8) && (_requested_coverage_samples > 0)) {
+      _requested_multisamples = 4;
+      _requested_coverage_samples = 8;
+    } else if (_requested_coverage_samples > 8) {
+      if (_requested_multisamples < 8) {
+        _requested_multisamples = 4;
+      } else {
+        _requested_multisamples = 8;
+      }
+      _requested_coverage_samples = 16;
+    }
+
+  } else {
+    _requested_coverage_samples = 0;
+  }
+#endif
+
+  if (_requested_multisamples > glgsg->_max_fb_samples) {
+    _requested_multisamples = glgsg->_max_fb_samples;
+  }
   _fb_properties.set_multisamples(_requested_multisamples);
+  _fb_properties.set_coverage_samples(_requested_coverage_samples);
 
   // Update aux settings to reflect the GL_MAX_DRAW_BUFFERS limit,
   // if we exceed it, that is.

panda/src/glxdisplay/glxGraphicsPipe.cxx

@@ -136,36 +136,29 @@ make_output(const string &name,
   // Second thing to try: a GLGraphicsBuffer
 
   if (retry == 1) {
-    if ((host==0)||
+    if (!gl_support_fbo || host == NULL ||
-        (!gl_support_fbo)||
+        (flags & (BF_require_parasite | BF_require_window)) != 0) {
-        ((flags&BF_require_parasite)!=0)||
-        ((flags&BF_require_window)!=0)) {
       return NULL;
     }
     // Early failure - if we are sure that this buffer WONT
     // meet specs, we can bail out early.
-    int _fbo_multisample = 0;
+    if ((flags & BF_fb_props_optional) == 0) {
-    if (!ConfigVariableBool("framebuffer-object-multisample", false, PRC_DESC("Enabled Multisample."))) {
+      if (fb_prop.get_indexed_color() ||
-      _fbo_multisample = 16;
+          fb_prop.get_back_buffers() > 0 ||
-    }
+          fb_prop.get_accum_bits() > 0) {
-    if ((flags & BF_fb_props_optional)==0) {
-      if ((fb_prop.get_indexed_color() > 0)||
-          (fb_prop.get_back_buffers() > 0)||
-          (fb_prop.get_accum_bits() > 0)||
-          (fb_prop.get_multisamples() > _fbo_multisample)) {
         return NULL;
       }
     }
+    if (posixgsg != NULL && posixgsg->is_valid() && !posixgsg->needs_reset()) {
+      if (!posixgsg->_supports_framebuffer_object ||
+          posixgsg->_glDrawBuffers == NULL) {
+        return NULL;
+      } else {
         // Early success - if we are sure that this buffer WILL
         // meet specs, we can precertify it.
-    if ((posixgsg != 0) &&
-        (posixgsg->is_valid()) &&
-        (!posixgsg->needs_reset()) &&
-        (posixgsg->_supports_framebuffer_object) &&
-        (posixgsg->_glDrawBuffers != 0)&&
-        (fb_prop.is_basic())) {
         precertify = true;
       }
+    }
     return new GLGraphicsBuffer(engine, this, name, fb_prop, win_prop,
                                 flags, gsg, host);
   }

panda/src/osxdisplay/osxGraphicsPipe.cxx

@@ -416,34 +416,29 @@ make_output(const string &name,
   // Second thing to try: a GLGraphicsBuffer
 
   if (retry == 1) {
-    if (!osx_support_gl_buffer) {
+    if (!osx_support_gl_buffer || !gl_support_fbo || host == NULL ||
-      return NULL;
+        (flags & (BF_require_parasite | BF_require_window)) != 0) {
-    }
-    if ((host==0)||
-        ((flags&BF_require_parasite)!=0)||
-        ((flags&BF_require_window)!=0)) {
       return NULL;
     }
     // Early failure - if we are sure that this buffer WONT
     // meet specs, we can bail out early.
-    if ((flags & BF_fb_props_optional)==0) {
+    if ((flags & BF_fb_props_optional) == 0) {
-      if ((fb_prop.get_indexed_color() > 0)||
+      if (fb_prop.get_indexed_color() ||
-          (fb_prop.get_back_buffers() > 0)||
+          fb_prop.get_back_buffers() > 0 ||
-          (fb_prop.get_accum_bits() > 0)||
+          fb_prop.get_accum_bits() > 0) {
-          (fb_prop.get_multisamples() > 0)) {
         return NULL;
       }
     }
+    if (posixgsg != NULL && posixgsg->is_valid() && !posixgsg->needs_reset()) {
+      if (!posixgsg->_supports_framebuffer_object ||
+          posixgsg->_glDrawBuffers == NULL) {
+        return NULL;
+      } else {
         // Early success - if we are sure that this buffer WILL
         // meet specs, we can precertify it.
-    if ((osxgsg != 0) &&
-        (osxgsg->is_valid()) &&
-        (!osxgsg->needs_reset()) &&
-        (osxgsg->_supports_framebuffer_object) &&
-        (osxgsg->_glDrawBuffers != 0)&&
-        (fb_prop.is_basic())) {
         precertify = true;
       }
+    }
     return new GLGraphicsBuffer(engine, this, name, fb_prop, win_prop, flags, gsg, host);
   }
   

panda/src/wgldisplay/wglGraphicsPipe.cxx

@@ -157,43 +157,29 @@ make_output(const string &name,
   // Second thing to try: a GLGraphicsBuffer
 
   if (retry == 1) {
-    if ((!gl_support_fbo)||
+    if (!gl_support_fbo || host == NULL ||
-        (host==0)||
+        (flags & (BF_require_parasite | BF_require_window)) != 0) {
-        ((flags&BF_require_parasite)!=0)||
-        ((flags&BF_require_window)!=0)) {
       return NULL;
     }
     // Early failure - if we are sure that this buffer WONT
     // meet specs, we can bail out early.
-    int _fbo_multisample = 0;
+    if ((flags & BF_fb_props_optional) == 0) {
-    if (!ConfigVariableBool("framebuffer-object-multisample", false, PRC_DESC("Enabled Multisample."))) {
-        _fbo_multisample = 16;
-    }
-    if ((flags & BF_fb_props_optional)==0) {
       if (fb_prop.get_indexed_color() ||
-          (fb_prop.get_back_buffers() > 0)||
+          fb_prop.get_back_buffers() > 0) ||
-          (fb_prop.get_accum_bits() > 0)||
+          fb_prop.get_accum_bits() > 0) {
-          (fb_prop.get_multisamples() > _fbo_multisample)) {
         return NULL;
       }
     }
-    if ((wglgsg != 0) &&
+    if (wglgsg != NULL && wglgsg->is_valid() && !wglgsg->needs_reset()) {
-        (wglgsg->is_valid()) &&
+      if (!wglgsg->_supports_framebuffer_object ||
-        (!wglgsg->needs_reset()) &&
+          wglgsg->_glDrawBuffers == NULL) {
-	!wglgsg->_supports_framebuffer_object) {
         return NULL;
-    }
+      } else {
-
         // Early success - if we are sure that this buffer WILL
         // meet specs, we can precertify it.
-    if ((wglgsg != 0) &&
-        (wglgsg->is_valid()) &&
-        (!wglgsg->needs_reset()) &&
-        (wglgsg->_supports_framebuffer_object) &&
-        (wglgsg->_glDrawBuffers != 0)&&
-        (fb_prop.is_basic())) {
         precertify = true;
       }
+    }
     return new GLGraphicsBuffer(engine, this, name, fb_prop, win_prop,
                                 flags, gsg, host);
   }