From b7f37b5e5a7a7437cfcfcf00bf48d081a82e6363 Mon Sep 17 00:00:00 2001
From: Ansgar Philippsen <ansgar.philippsen@gmail.com>
Date: Thu, 1 Nov 2012 14:00:11 -0400
Subject: [PATCH] added multisample anti-aliasing to export; fixed memory leak
 in offscreen buffer

---
 modules/gfx/pymod/export_scene.cc            | 12 ++--
 modules/gfx/src/entity.cc                    |  3 +-
 modules/gfx/src/gfx_node.cc                  |  4 +-
 modules/gfx/src/impl/glx_offscreen_buffer.cc | 59 ++++++++++++++------
 modules/gfx/src/scene.cc                     | 26 ++++++++-
 modules/gfx/src/scene.hh                     | 15 ++++-
 6 files changed, 91 insertions(+), 28 deletions(-)

diff --git a/modules/gfx/pymod/export_scene.cc b/modules/gfx/pymod/export_scene.cc
index 9f3042cb2..5a8a5929d 100644
--- a/modules/gfx/pymod/export_scene.cc
+++ b/modules/gfx/pymod/export_scene.cc
@@ -83,13 +83,14 @@ void export_Scene()
   void (Scene::* set_light_prop2)(float,float,float) = &Scene::SetLightProp;
 
   void (Scene::* export1)(const String&, uint, uint, bool) = &Scene::Export;
-  void (Scene::* export2)(const String&, bool) = &Scene::Export;
-  void (Scene::* export3)(Exporter*) const = &Scene::Export;
+  void (Scene::* export2)(const String&, uint, uint, int, bool) = &Scene::Export;
+  void (Scene::* export3)(const String&, bool) = &Scene::Export;
+  void (Scene::* export4)(Exporter*) const = &Scene::Export;
   void (Scene::*remove1)(const GfxNodeP&) = &Scene::Remove;
   void (Scene::*remove2)(const String&) = &Scene::Remove;
   void (Scene::*center_on1)(const String&) = &Scene::CenterOn;
   void (Scene::*center_on2)(const GfxObjP&) = &Scene::CenterOn;
-  
+  bool (Scene::*start_offscreen_mode2)(unsigned int, unsigned int, int) = &Scene::StartOffscreenMode;  
   class_<Viewport>("Viewport", init<>())
     .def_readwrite("x", &Viewport::x)
     .def_readwrite("y", &Viewport::y)
@@ -197,7 +198,8 @@ void export_Scene()
     .def("Apply", apply)
     .def("Export",export1, arg("transparent")=false)
     .def("Export",export2, arg("transparent")=false)
-    .def("Export",export3)
+    .def("Export",export3, arg("transparent")=false)
+    .def("Export",export4)
     .def("ExportPov",&Scene::ExportPov,
          scene_export_pov_overloads())
     .def("PushView",&Scene::PushView)
@@ -227,7 +229,7 @@ void export_Scene()
     .add_property("ao_quality",&Scene::GetAmbientOcclusionQuality,&Scene::SetAmbientOcclusionQuality)
     .add_property("ao_size",&Scene::GetAmbientOcclusionSize,&Scene::SetAmbientOcclusionSize)
     .def("AttachObserver",&Scene::AttachObserver)
-    .def("StartOffscreenMode",&Scene::StartOffscreenMode)
+    .def("StartOffscreenMode",start_offscreen_mode2)
     .def("StopOffscreenMode",&Scene::StopOffscreenMode)
     .def("SetShadingMode",&Scene::SetShadingMode)
     .def("SetBeacon",&Scene::SetBeacon)
diff --git a/modules/gfx/src/entity.cc b/modules/gfx/src/entity.cc
index df1ad6bfc..877540178 100644
--- a/modules/gfx/src/entity.cc
+++ b/modules/gfx/src/entity.cc
@@ -376,8 +376,9 @@ void Entity::CustomRenderPov(PovState& pov)
 
 void Entity::Export(Exporter* ex)
 {
+  if(!IsVisible()) return;
+
   ex->NodeStart(GetName(),Exporter::OBJ);
-  // in the simplest case, just export va
   if(rebuild_ || refresh_) {
     PreRenderGL(true);
   }
diff --git a/modules/gfx/src/gfx_node.cc b/modules/gfx/src/gfx_node.cc
index 813ce6f41..59b44d162 100644
--- a/modules/gfx/src/gfx_node.cc
+++ b/modules/gfx/src/gfx_node.cc
@@ -108,7 +108,9 @@ void GfxNode::Export(Exporter* ex)
   if(!IsVisible()) return;
   ex->NodeStart(GetName(),Exporter::GROUP);
   for(GfxNodeVector::iterator it=node_vector_.begin();it!=node_vector_.end();++it) {
-    (*it)->Export(ex);
+    if((*it)->IsVisible()) {
+      (*it)->Export(ex);
+    }
   }
   ex->NodeEnd(GetName());
 }
diff --git a/modules/gfx/src/impl/glx_offscreen_buffer.cc b/modules/gfx/src/impl/glx_offscreen_buffer.cc
index 10fcb12af..6b934d115 100644
--- a/modules/gfx/src/impl/glx_offscreen_buffer.cc
+++ b/modules/gfx/src/impl/glx_offscreen_buffer.cc
@@ -21,6 +21,8 @@
   Author: Ansgar Philippsen
 */
 
+#include <iomanip>
+
 #include <ost/log.hh>
 #include <ost/gfx/offscreen_buffer.hh>
 #include <ost/gfx/scene.hh>
@@ -63,40 +65,57 @@ OffscreenBuffer::OffscreenBuffer(unsigned int width, unsigned int height, const
   attrib_list.push_back(GLX_STENCIL_SIZE);
   attrib_list.push_back(8);
   attrib_list.push_back(GLX_SAMPLE_BUFFERS);
-  attrib_list.push_back(1);
+  attrib_list.push_back(f.multisample ? 1 : 0);
   attrib_list.push_back(0);
-
+  bool search_multisample=f.multisample;
   int nelem=0;
   LOG_DEBUG("offscreen buffer: glXChooseFBConfig");
   fbconfig_ =glXChooseFBConfig(dpy_,0,&attrib_list[0],&nelem);
   if(fbconfig_==0 || nelem==0) {
-    LOG_DEBUG("no offscreen rendering context with multisample, trying without");
-    // take out the multisample requirement
-    attrib_list[attrib_list.size()-2]=0;
-    fbconfig_ =glXChooseFBConfig(dpy_,0,&attrib_list[0],&nelem);
-    if(fbconfig_==0 || nelem==0) {
+    if(f.multisample) {
+      LOG_DEBUG("no offscreen rendering context with multisample, trying without");
+      // take out the multisample requirement
+      attrib_list[attrib_list.size()-2]=0;
+      fbconfig_ =glXChooseFBConfig(dpy_,0,&attrib_list[0],&nelem);
+      if(fbconfig_==0 || nelem==0) {
+        LOG_ERROR("error creating offscreen rendering context: glXChooseFBConfig failed");
+        return;
+      }
+      search_multisample=false;
+    } else {
       LOG_ERROR("error creating offscreen rendering context: glXChooseFBConfig failed");
+      XCloseDisplay(dpy_);
       return;
     }
   }
-#if 0
-  /*
-   let export routine and start offscreen rendering context ask for min/max/next available
-   multisample
-   */
-  for(size_t i=0;i<nelem;++i) {
-    std::cerr << i << " ";
+#ifndef NDEBUG
+  LOG_VERBOSE("offscreen buffer: available framebuffer configs");
+  for(int i=0;i<nelem;++i) {
     int rbits; glXGetFBConfigAttrib(dpy_,fbconfig_[i], GLX_RED_SIZE, &rbits);
     int gbits; glXGetFBConfigAttrib(dpy_,fbconfig_[i], GLX_GREEN_SIZE, &gbits);
     int bbits; glXGetFBConfigAttrib(dpy_,fbconfig_[i], GLX_BLUE_SIZE, &bbits);
     int dbits; glXGetFBConfigAttrib(dpy_,fbconfig_[i], GLX_DEPTH_SIZE, &dbits);
     int sbits; glXGetFBConfigAttrib(dpy_,fbconfig_[i], GLX_STENCIL_SIZE, &sbits);
     int ms; glXGetFBConfigAttrib(dpy_,fbconfig_[i], GLX_SAMPLES, &ms);
-    std::cerr << "rgb=" << rbits << "." << gbits << "." << bbits << " d=" << dbits << " s=" << sbits << " ms=" << ms << std::endl;
+    LOG_VERBOSE(" " << std::setw(3) << i << std::setw(0) << ": rgb=" << rbits << "." << gbits << "." << bbits << " d=" << dbits << " s=" << sbits << " ms=" << ms);
   }
-  fb_config_id_=nelem-1;
 #endif
   fb_config_id_=0;
+  if(search_multisample) {
+    int ms;
+    int best_ms;
+    glXGetFBConfigAttrib(dpy_,fbconfig_[0], GLX_SAMPLES, &best_ms);
+    for(int i=1;i<nelem;++i) {
+      glXGetFBConfigAttrib(dpy_,fbconfig_[i], GLX_SAMPLES, &ms);
+      if(ms<=static_cast<int>(f.samples) && ms>best_ms) {
+        fb_config_id_=i;
+        best_ms=ms;
+      }
+    }
+  }
+#ifndef NDEBUG
+  LOG_VERBOSE("offscreen buffer: using buffer # " << fb_config_id_ << " for export");
+#endif
 
   attrib_list.clear();
   attrib_list.push_back(GLX_PBUFFER_WIDTH);
@@ -109,6 +128,8 @@ OffscreenBuffer::OffscreenBuffer(unsigned int width, unsigned int height, const
   pbuffer_ = glXCreatePbuffer(dpy_, fbconfig_[fb_config_id_], &attrib_list[0]);
   if(!pbuffer_) {
     LOG_ERROR("error creating offscreen rendering context: glXCreatePBuffer failed");
+    XFree(fbconfig_);
+    XCloseDisplay(dpy_);
     return;
   }
 
@@ -126,6 +147,8 @@ OffscreenBuffer::OffscreenBuffer(unsigned int width, unsigned int height, const
   if(!context_) {
     LOG_ERROR("error creating offscreen rendering context: glXCreateNewContext failed");
     glXDestroyPbuffer(dpy_, pbuffer_);
+    XFree(fbconfig_);
+    XCloseDisplay(dpy_);
     return;
   }
 
@@ -139,6 +162,10 @@ OffscreenBuffer::~OffscreenBuffer()
     glXDestroyContext(dpy_, context_);
     LOG_DEBUG("offscreen buffer: glXDestroyPbuffer()");
     glXDestroyPbuffer(dpy_, pbuffer_);
+    LOG_DEBUG("offscreen buffer: XFree(fbconfig_list)");
+    XFree(fbconfig_);
+    LOG_DEBUG("offscreen buffer: XCloseDisplay()");
+    XCloseDisplay(dpy_);
   }
 }
 
diff --git a/modules/gfx/src/scene.cc b/modules/gfx/src/scene.cc
index e4d37b841..724482a79 100644
--- a/modules/gfx/src/scene.cc
+++ b/modules/gfx/src/scene.cc
@@ -1584,11 +1584,18 @@ uint Scene::GetSelectionMode() const
   return selection_mode_;
 }
 
-bool Scene::StartOffscreenMode(unsigned int width, unsigned int height)
+bool Scene::StartOffscreenMode(unsigned int width, unsigned int height, int max_samples)
 {
   LOG_DEBUG("Scene: starting offscreen rendering mode " << width << "x" << height);
   if(main_offscreen_buffer_) return false;
-  main_offscreen_buffer_ = new OffscreenBuffer(width,height,OffscreenBufferFormat(),true);
+  OffscreenBufferFormat obf;
+  if(max_samples>0) {
+    obf.multisample=true;
+    obf.samples=max_samples;
+  } else {
+    obf.multisample=false;
+  }
+  main_offscreen_buffer_ = new OffscreenBuffer(width,height,obf,true);
 
   if(!main_offscreen_buffer_->IsValid()) {
     LOG_ERROR("Scene: error during offscreen buffer creation");
@@ -1642,6 +1649,12 @@ void Scene::StopOffscreenMode()
 
 void Scene::Export(const String& fname, unsigned int width,
                    unsigned int height, bool transparent)
+{
+  Export(fname,width,height,0,transparent);
+}
+
+void Scene::Export(const String& fname, unsigned int width,
+                   unsigned int height, int max_samples, bool transparent)
 {
   int d_index=fname.rfind('.');
   if (d_index==-1) {
@@ -1658,7 +1671,14 @@ void Scene::Export(const String& fname, unsigned int width,
 
   // only switch if offscreen mode is not active
   if(of_flag) {
-    if(!StartOffscreenMode(width,height)) {
+    if(max_samples<0) {
+      int msamples=0;
+      if(OST_GL_VERSION_2_0) {
+        glGetIntegerv(GL_SAMPLES, &msamples);
+      }
+      max_samples=msamples;
+    }
+    if(!StartOffscreenMode(width,height, max_samples)) {
       return;
     }
   }
diff --git a/modules/gfx/src/scene.hh b/modules/gfx/src/scene.hh
index a8a9afbe1..477b160ca 100644
--- a/modules/gfx/src/scene.hh
+++ b/modules/gfx/src/scene.hh
@@ -294,6 +294,10 @@ class DLLEXPORT_OST_GFX Scene {
   /// dimensions here are ignored
   void Export(const String& fname, unsigned int w,
               unsigned int h, bool transparent=false);
+  /// \brief export into bitmap, using multisample anti-aliasing
+  /// \ref Scene::StartOfffscreenMode(unsigned int, unsigned int, int) for more detail
+  void Export(const String& fname, unsigned int w,
+              unsigned int h, int max_samples, bool transparent=false);
 
   /// \brief export snapshot of current scene
   void Export(const String& fname, bool transparent=false);
@@ -465,9 +469,16 @@ class DLLEXPORT_OST_GFX Scene {
     During batch mode, this is the only way to get meaningful
     functionality with the gfx module
 
-    returns true upon success and false upon failure
+    Returns true upon success and false upon failure
+
+    You can ask for multisampling to be enabled by giving the
+    max_samples a value larger than zero; in this case, the framebuffer
+    with at most this many samplebuffers will be used. The recommended
+    value here is 4; going to 8 or 16 may give you higher export times
+    with usually no marked increase in quality.
+
   */
-  bool StartOffscreenMode(unsigned int w, unsigned int h);
+  bool StartOffscreenMode(unsigned int w, unsigned int h, int max_samples);
   /// \brief stops offline rendering in interactive mode
   void StopOffscreenMode();
 
-- 
GitLab